-2

This question already has an answer here:

Often I came across situations like this, how to write this code in a neat and clean way. One more issue I find here is performance as I am iteration a list and then it's properties.

Edit : - while the suggested How to tackle a 'branched' arrowhead anti-pattern? post address the nested if-elseif-if concern to a great extends. Still, I am looking for an answer for how to handle multiple try catch in (if any) in the code, and how to improve performance.

for (String item : someList) {
    IResultOne resultOne = doSomething(item);

    if (resultOne != null) {
        for (SomeObject obj :resultOne.getSomeOtherList) {
            IResultTwo resultTwo = doMegaProcessing(obj);

            if(resultTwo != null) {
                try {
                    doSomeMoreProcessing(resultTwo);
                } 
                catch (CanNotProcessingException ex) {
                    errorObj.addError("can't process reason");
                }
            } 
            else {
                errorObj.addError("item does not have {} error ");
            }
        }
    } 
    else {
        errorObj.addError("item is invalid "+item)
    }
} 

marked as duplicate by Robert Harvey, gnat, Greg Burghardt, amon, JimmyJames Jun 27 '18 at 21:21

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

  • 2
    For what it's worth, cleaning up your code formatting will help tremendously. See my edit to your code. – Robert Harvey Jun 27 '18 at 18:48
  • @RobertHarvey Yes...it looks cleaner now... thanks – SumanP Jun 27 '18 at 18:51
  • Removing the obvious syntax errors also helps (did so). That could make your question eligible for codereview.stackexchange (AFAIK they don't accept code which does not compile). – Doc Brown Jun 27 '18 at 18:52
  • Don't know how tractable this is in Java, but promises and continuations would make this a lot cleaner. – Robert Harvey Jun 27 '18 at 18:58
  • 1
    Is there a reason for the javascript tag? I also think this would be better for codereview.stackexchange – Hangman4358 Jun 27 '18 at 20:00
1

First, I would use sentinel conditions instead of nested those if/else blocks. That reduces the indentation, and will already make the code block more readable. Second, the try/catch block is probably better refactored to a function which directly returns just the error string instead of throwing an exception.

In total, this leads to something like this:

for (String item : someList) {
    IResultOne resultOne = doSomething(item);
    if (resultOne == null) {
        errorObj.addError("item is invalid "+item)
        continue;
    }
    // ...
    for (SomeObject obj :resultOne.getSomeOtherList) {
        IResultTwo resultTwo = doMegaProcessing(obj);
        if(resultTwo == null) {
            errorObj.addError("item does not have {} error ");
            continue;
        }

        String errorText = trySomeMoreProcessing(resultTwo);
        if(errorText!=null)
            errorObj.addError(errorText);
    }
}

 String trySomeMoreProcessing(IResultTwo resultTwo) {
     try {
        doSomeMoreProcessing(resultTwo);
     } 
     catch (CanNotProcessingException ex) {
          return "can't process reason";
     }
     return null;
 }

Furthermore, I would consider to refactor parts like the inner loop to a function on its own, and give that function a sensible name.

  • Thanks.. This looks cleaner to read and is helpful indeed – SumanP Jun 27 '18 at 19:16
  • 1
    Btw I read the question, noticed gnat's duplicate suggestion, remembered the typical refactoring pattern for that problem, and was then surprised to read exactly the code I was thinking about in your answer. I take this as a strong indication that gnat's dupe was very much not “nonsense” (at least in this case, I understand your general frustration). – amon Jun 27 '18 at 20:28
  • @amon: ok, I admit, after reading the answer again, I can see some similarities (but probably far too few to be really useful for the OP of this question). – Doc Brown Jun 27 '18 at 20:50
  • I agree with the first part, but I think the try-catch is fine. (Exceptions exist for a reason!) – Solomon Ucko Jun 27 '18 at 21:18
  • @SolomonUcko: you missed my point, I did not suggest to remove the try-catch, I suggested to refactor the mapping from try-catch to a string into a separate function (see my edit). There is no change in semantics to the original code. – Doc Brown Jun 28 '18 at 5:26

Not the answer you're looking for? Browse other questions tagged or ask your own question.