asked on

Best practice to using if - else logic

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

David Kroll

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

jkteater

ASKER

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

ASKER CERTIFIED SOLUTION

David Kroll

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

jkteater

ASKER

Nice - thanks

mccarl

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)