I am debugging one legacy J2EE application I and think I found some dead code and some redundant condition checks. Please give me your thoughts on my observations i.e. if they are valid. Please note that I am not permitted to rewrite this block, only to fix something if wrong or redundant....else if ((emailList != null) && (emailTest != null)                   && (emailTest.compareTo(IParameterNames.EMAIL_GO) == 0))  // #1 One of the AND conditions to enter this else if block is: emailList != null,              {                                                             // #2 so if emailList != null is not true we wouldn't even be in this else if block.                response.getOutputStream().print(...blah blah blah...);                response.getOutputStream().print(...blah blah blah...);                response.getOutputStream().print(...blah blah blah...);                if (emailList != null)  //#3 Here we check again if primary condition is fulfilled: emailList != null.                                         //#4 If emailList != null is not true we wouldn't even be in this else if block.This should be removed IMHO                {                  response.getOutputStream().print(emailList.size());                }                else //#5 And as emailList != null was true at the else if here it can't be different than that. This should be removed IMHO as it is DEAD CODE.                {                  response.getOutputStream().print(0 );                }                response.getOutputStream().print(...blah blah blah...);                response.getOutputStream().print(...blah blah blah...);                if (emailList != null)  //#6 Here we check again if primary condition is fulfilled: emailList != null.                                         //#7 If emailList != null is not true we wouldn't even be in this else if block.This should be removed IMHO                {                  response                      .getOutputStream()                      .print(...blah blah blah...);                  for (int i = 0; i < emailList.size(); i++)                  {                    email = (EmailMessage) emailList.get(i);                    if (email != null)                    {                      response.getOutputStream().print(outputEmailAddressAsHTML(email, i).toString());                    }                  }                }                response.getOutputStream().print(..lots.of.blah blah blah...);              }...           
Legacy application unreachable code
java
Your assessment is correct, the emailList null checks are redundant in that block.I would recommend that you use the FindBugs tool to perform static analysis on your code. It is available as a plugin to Eclipse, and other tools too. FindBugs would clearly indicate and mark all instances where these problems happen, and also a bunch of other things too.Additionally, I would neaten things up by pulling just one final local variable for the response.getOutputStream() too. Additionally, you should be using generics, and enhanced-for (for-each) loops over your emailList.Summarizing this would be:...else if ((emailList != null) && (emailTest != null)               && (emailTest.compareTo(IParameterNames.EMAIL_GO) == 0))                        {            PrintOutputStream respos = response.getOutputStream();            respos.print(...blah blah blah...);            respos.print(...blah blah blah...);            respos.print(...blah blah blah...);            respos.print(emailList.size());            respos.print(...blah blah blah...);            respos.print(...blah blah blah...);            respos.print(...blah blah blah...);            for (EmailMessage email : emailList) {                if (email != null) {                  respos.print(outputEmailAddressAsHTML(email, i).toString());                }            }            respos.print(..lots.of.blah blah blah...);          }