0

I need to reduced following java method complexity according to sonar acceptable level. now ,it given this like sonar issue.

enter image description here

need some expert help to do this.

public List<X> Y(final DateTime treatmentDiscontinueTime,
                                                    final List< P> validPrescribedPrescriptions)
  {
    final List<x> doseWrapperList = new ArrayList<>();
    final int noOfPrescriptions = validPrescribedPrescriptions.size();

    for (int prescriptionIndex = 0; prescriptionIndex < noOfPrescriptions; prescriptionIndex++)
    {
      final BasePrescribedPrescription basePrescribedPrescription = validPrescribedPrescriptions.get(prescriptionIndex);
      final String firstDoseText = basePrescribedPrescription.getFirstText();
      final String secondDoseText = basePrescribedPrescription.getSecondText();
      final boolean accordingToSchedule = A.ACCORDING.equals(firstDoseText);
      final boolean specificPrescription = A.SP.equals(firstDoseText);
      final boolean specificVbTypePrescription = A.SPVB.equals(firstDoseText);

      List<D> doseDetails = new ArrayList<>(basePrescribedPrescription.getDoseDetails());


      final DateTime changedDosageEndDate =
        getChangedDoseEndDate(basePrescribedPrescription.getActualTerminateDate(), treatmentDiscontinueTime);
      final int noOfDosages = doseDetails.size();

      for (int doseIndex = 0; doseIndex < noOfDosages; doseIndex++)
      {
        final D doseDetail = doseDetails.get(doseIndex);

        if ((doseDetail.getStart().getStartDate() != null) && (changedDosageEndDate != null) &&
            doseDetail.getStart().getStartDate().isAfter(changedDosageEndDate))
        {
          continue;
        }

        String previewDoseText;

        if (accordingToSchedule)
        {
          previewDoseText = X
        }
        else if (specificPrescription)
        {
          previewDoseText = Y;
        }
        else if (specificVbTypePrescription)
        {
          previewDoseText = Z;
        }
        else if (noOfDosages == 2)
        {

          previewDoseText = ((doseIndex == 0) ? secondDoseText : firstDoseText);
        }
        else
        {
          previewDoseText = firstDoseText;
        }

        final boolean isUnplanned =isuNplaned()



        if (!isUnplanned)
        {
          doseStart = getStartDate();
          doseEnd = getEndDate();
        }


        doseWrapperList.add(new DoseInfoLiteDTOWrapper(previewDoseText, doseStart, doseEnd, doseDetail));

      }
    }
    return doseWrapperList;
  }

i need some expert help to resoled this sonar issue. I thing different way to extract code fragment , breakdown this method to little parts.but still couldn't find some proper way to do it.

uma
  • 1,477
  • 3
  • 32
  • 63
  • 3
    If the code works, please head over to [codereview.se] to ask instead. – Andy Turner Jul 16 '19 at 14:53
  • 1
    Hint: only use final where it makes sense. Attaching it to each and any variable creates a lot of useless line noise. – GhostCat Jul 16 '19 at 14:57
  • 1
    One thing you could try is to move that huge if-block into a method like `previewDoseText = theNewMethod()`. – Thomas Jul 16 '19 at 14:58
  • 1
    "`previewDoseText = ((doseIndex == 0) ? secondDoseText : firstDoseText);`" that looks wrong... surely if it's the zeroth dose, that's not the second dose? – Andy Turner Jul 16 '19 at 14:59
  • @AndyTurner Thank you for point out. code is working , i need just reduced complexity. – uma Jul 16 '19 at 14:59
  • 1
    @uma it might *work*, but it looks wrong, and thus increases the cognitive burden. – Andy Turner Jul 16 '19 at 15:01
  • @AndyTurner i will checked it. yep ,every time it became false. ??? – uma Jul 16 '19 at 15:02
  • 1
    @GhostCat What do you mean "where it makes sense"? Where does it not make sense? I use it on every variable I don't intend to reassign. – Michael Jul 16 '19 at 15:03
  • 1
    @Michael "clean code" advocates short methods which you can easily" dissect" within a few seconds. Why adding final to a variable that is just read, or assigned just once? In other words : code that *really* benefits from final on local variables or parameters has most likely quality problems already. I almost never use final for non fields, and the code I see that does... would much more benefit from other activities. If your methods are so complicated that you accidentally assign something to the wrong variable, then adding final is a work around, not solving the root problem! – GhostCat Jul 16 '19 at 17:04
  • 1
    @GhostCat "*Why adding final to a variable that is just read, or assigned just once*" To make explicit my intention to not reassign it. To the compiler, to myself, and to anyone else reading it. It seems like your opinion is that final is never useful for locals (since final can *only* be applied to variables "*just read, or assigned just once*"). Which is fine, though I disagree. But in such case, your comment about "where it makes sense" does not convey that sentiment. It is certainly verbose but it is not noise, since it axiomatically does convey some information. – Michael Jul 16 '19 at 17:14
  • It’s a strange pattern, to create loads of local variables for things you use exactly one time, but to use repeated code, like `doseDetail.getStart().getStartDate()`, where a local variable could actually improve the code. Besides that, there is no reason to copy `basePrescribedPrescription.getDoseDetails()` into an `ArrayList` nor to use an index based loop. Just use `for(final D doseDetail: basePrescribedPrescription.getDoseDetails()) …`; the only place where you use the index, `doseIndex == 0`, can be replaced with `doseWrapperList.isEmpty()`, to find out whether it is the first element. – Holger Jul 17 '19 at 11:20

1 Answers1

1

It's not difficult to clear, i think:

  • Use simple For loop
  • Create more small methods to do small(clear) things for For loop
  • Block if/esle: use simple statement

Hint: Study TDD to write clean code as possible

tony2011
  • 13
  • 2
  • 1
    Just for the record: one can follow clean code without tdd. But tdd done right sure helps! – GhostCat Jul 16 '19 at 18:36
  • yes, there are many ways to do that, and tdd isnt difficult to study but isnot easy to apply. For me, combine everything as possible: + OOP + Design pattern + SOLID + TDD .... – tony2011 Jul 17 '19 at 08:42