We help IT Professionals succeed at work.

Best practice to using if - else logic

jkteater
jkteater asked
on
In my method I am getting a String value for XMLFormat.  If the value is null or has no value I want to run method a, if the value has a value of version 1, I want to run method a.  but if the value has a value of version 2, I want to run method b.  Please tell me if my logic looks OK?

public void setXMLFormat(){
      TCPreferenceService ups = session.getPreferenceService(); 
      String xmlFormat = ups.getString(TCPreferenceService.TC_preference_user,  XML_FORMAT");    
      if(xmlFormat == null || xmlFormat.trim().isEmpty()) {
         buildXMLTwo(true);
      }
      else if(xmlFormat == "version 1") {
         buildXMLTwo(true);       
      }
      else {
         buildXml(true);
      }
   }

Open in new window

Comment
Watch Question

I wouldn't put a separate else statement for "version 1", since it's going to the same method.

public void setXMLFormat(){
      TCPreferenceService ups = session.getPreferenceService(); 
      String xmlFormat = ups.getString(TCPreferenceService.TC_preference_user,  XML_FORMAT");    
      if(xmlFormat == null || xmlFormat.trim().isEmpty() || xmlFormat == "version 1" ) {
         buildXMLTwo(true);
      }
      else {
         buildXml(true);
      }
   }
                                  

Open in new window

Author

Commented:
Is it possible to condense these 3 If statements?  I will increase the points being I am asking somewhat a new question

 if ((value == null || value.trim().isEmpty()) && isMatrixPRLCellReadOnly == true) { 
                prlValue = false;
                break;
             }          
                        
             if ((value == null || value.trim().isEmpty()) && value1 == null) {    
                prlValue = false;
                break;
             }
             
             if ((value == null || value.trim().isEmpty()) && value1.trim().isEmpty()) {    
                prlValue = false;
                break;
             }

Open in new window

Since you're checking for value == null or value.trim().isEmpty in all three conditions, you can condense the other three conditions inside of the master condition.

if ((value == null || value.trim().isEmpty()){
  if isMatrixPRLCellReadOnly == true { 
                prlValue = false;
                break;
             }          
  if value1 == null) {    
                prlValue = false;
                break;
             }
  if value1.trim().isEmpty() {    
                prlValue = false;
                break;
             }
}

Open in new window

Author

Commented:
Nice - thanks
mccarlIT Business Systems Analyst / Software Developer
SILVER EXPERT
Top Expert 2015

Commented:
Shame you already awarded points...

Two things to say...

xmlFormat == "version 1"
is incorrect and WON'T do what you want. Use xmlFormat.equals("version 1") instead.


Is it possible to condense these 3 If statements?
Yes you can go even further than the accepted answer, since the the contents inside each of the 3 'if' statements are the same...

 if (
      (value == null || value.trim().isEmpty()) 
          && 
      (isMatrixPRLCellReadOnly == true || value1 == null || value1.trim().isEmpty())
    ) {    
          prlValue = false;
          break;
      }

Open in new window

(Note: obviously, the whitespace is just for clarity in this answer)

Explore More ContentExplore courses, solutions, and other research materials related to this topic.