[Okta Webinar] Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 426
  • Last Modified:

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

0
jkteater
Asked:
jkteater
  • 2
  • 2
1 Solution
 
David KrollCommented:
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

0
 
jkteaterAuthor 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

0
 
David KrollCommented:
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

0
 
jkteaterAuthor Commented:
Nice - thanks
0
 
mccarlIT Business Systems Analyst / Software DeveloperCommented:
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)
0

Featured Post

Free Tool: SSL Checker

Scans your site and returns information about your SSL implementation and certificate. Helpful for debugging and validating your SSL configuration.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

  • 2
  • 2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now