Wednesday, 31 March 2010

When Order of Precedence goes bad

Hi all,

Well I told a lie about posting every week, but hey I was enjoying the sunshine in the antipodean lands (Australia and NZ)! So from here on in I'll try to keep my original promise and write _something_ every week and so here we are! Warning this post is heavily aimed at the Java world, those who prefer a non technical read had best leave now :-).

So a colleague and I were tracking down a problem late last night and were looking at a particular line of code that wasn't executing in the order that the original author intended:

if (this.conditionOne() && (this.conditionTwo() || this.conditionThree()))

Basically an assumption was made that the(this.conditionTwo() || this.conditionThree()) would be evaluated first.  We were reminded that the BEDMAS rules of arithmetic do not necessarily apply to Java order of precedence when evaluating expressions. Feel free to run the experiment below if you, like us, wanted a reminder....

package org.martijnverburg.experiments;

/**
 * This class runs experiments to prove various order 
 * of Precedence rules in the JDK
 *
 * The use of System.out.println is generally frowned 
 * upon, but I'll forgive myself in this case.
 * 
 * This probably should be a nice Unit test, many 
 * apologies!
 *
 * @author Martijn Verburg (aka karianna)
 */
public class PrecedenceExperiments
{

  /**
   * Fire off the experiments
   *
   * @param args - Not used
   */
  public static void main(String[] args)
  {
    PrecedenceExperiments test = new PrecedenceExperiments();
    test.bracketedAndOrExperiment();
  }


  /**
   * This method tests an assumption that was made 
   * in any code base about the order of precedence 
   * when (x && (y || z)) gets executed 
   * as opposed when ((y || z) && x) gets 
   * executed.
   */
   private void bracketedAndOrExperiment()
   {

     // Run ((y || z) && x)
     if ((this.conditionTwo() || this.conditionThree()) 
          && this.conditionOne())
     {
       System.out.println("Completed case: ((y || z) && x)");
     }



     // Run (x && (y || z)) case
     if (this.conditionOne() && 
          (this.conditionTwo() || this.conditionThree()))
     {
       System.out.println("Completed case: (x && (y || z))");
     }

   }

   private boolean conditionOne()
   {
     System.out.println("1");
     return true;
   }

   private boolean conditionTwo()
   {
     System.out.println("2");
     return true;
   }

   private boolean conditionThree()
   {
     System.out.println("3");
     return true;
   }
}

So are we there yet?  Well I guess for the code base I'm currently working on that would be a no, we've a few more of these cases to discover and iron out.  Another musing that comes out of this is that perhaps  "maintainability" is sometimes better than code brevity, maybe writing code in the style of if (x && (y || z)) can be simplified to it's component parts.

Sometimes it's the little things.

4 comments:

  1. aha - the joys of the 'short circuit' OR and AND operators!

    They are named short circuit as they aim to be efficient in not evaluating any unnecessary expressions when the outcome of the overall expression can already be determined.

    The && short circuit operator requires both sides of the expression to evaluate to true if the statement body is to be entered. To do this efficiently it always evaluates the left side first, if this resolves to false then the operator doesn't bother evaluating the right hand side since it already knows that the overall expression cannot be true.

    The || short circuit operator only requires one side of the expression to be true in order to enter the if statement. It also evaluates the left side first, if this evaluates to true then it doesn't need to evaluate the right hand side as it already knows it can enter the statement body. However if the left side evaluates to false then it must evaluate the right hand side to find out if the statement body can be entered.

    You probably know all this but it's fresh in my mind as I've been re-reading the SCJP book recently so thought I'd share in case it's of any use :-)

    Cheers,

    Edd

    ReplyDelete
  2. Hi Edd,

    So very right you are! It's one of those things that you study when you first learn Java. If you're like me ~10 years or so down the track you find yourself writing an experiment to prove what you just re-checked in your Java in a Nutshell book ;).

    ReplyDelete
  3. Hi Martijn,

    Can you share why the order was important for the code you were looking at? (without getting into specifics).

    Was it a case of the computational cost of the "conditionOne" method, or was there some side-effect of the "conditionTwo" or "conditionThree" method?

    Changing the order could be fixing the immediate problem, but delaying a candidate for deeper refactoring.

    ReplyDelete
  4. Hi Stephen, yes it was a side-effect of the conditionThree method (thread locking with a process in condtionOne). Does this mean we have a code smell? Undoubtedly yes :), we've left the JIRA issue open for further investigation and a clean re-factoring.

    ReplyDelete