Where do conditionals come from?

Given that we want to reduce the number of conditional branches in our code, I wonder whether it is possible to catalogue all of the reasons they exist, so that we might then in turn list a few ways to eliminate each category.

So to that end I’ve listed out those origins of conditionals that I could think of. Is this a fool’s errand? Possibly, but let’s give it a try anyway. Please comment if you can spot any that I have missed, and let’s see if we can grow a “complete” list…

  1. Checking a value returned to me from code I own
    This is the essence of the Null Check smell, and more generally Connascence of Meaning. For example, see this (intentionally bad) code from William C. Wake’s Refactoring Workbook:

    public class Report {
      public static void report(Writer out, List<Machine> machines, Robot robot) throws IOException
      {
        //...
    
        out.write("Robot");
        if (robot.location() != null)
          out.write(" location=" + robot.location().name());
    
        if (robot.bin() != null)
          out.write(" bin=" + robot.bin());
    
        //...
      }
    }
    
  2. Checking a value returned to me from code I do not own
    For example, in Rails projects we often see code of this kind:

    if User.exists?(:user_id => current_user.id)
      # ...
    else
      # ...
    end
    
  3. Checking a parameter passed to me by code I own
    Whenever I write a method that accepts a Boolean parameter, I open myself up to the Control Couple smell. Here’s an example from Piotr Solnica’s blog:

    def say(sentence, loud = false)
      if loud
        puts sentence.upcase
      else
        puts sentence
      end
    end
    
  4. Checking a parameter passed to me by code I do not own
    Here’s an example from the jQuery documentation:

    <script>
    var xTriggered = 0;
    $( "#target" ).keypress(function( event ) {
      if ( event.which == 13 ) {
         event.preventDefault();
      }
      xTriggered++;
      var msg = "Handler for .keypress() called " + xTriggered + " time(s).";
      $.print( msg, "html" );
      $.print( event );
    });
    </script>
    
  5. Checking my own state or attributes
    Here’s an (intentionally bad) example from Martin Fowler’s Refactoring:

    class Employee {
      private int _type;
      static final int ENGINEER = 0;
      static final int SALESMAN = 1;
      static final int MANAGER = 2;
    
      Employee (int type) {
        _type = type;
      }
    
      int payAmount() {
        switch (_type) {
          case ENGINEER:
            return _monthlySalary;
          case SALESMAN:
            return _monthlySalary + _commission;
          case MANAGER:
            return _monthlySalary + _bonus;
          default:
            throw new RuntimeException("Incorrect Employee");
        }
      }
    }
    
  6. Checking a value I set previously in this method
    For example:

    public String format(List<String> items) {
      StringBuffer s = new StringBuffer();
      boolean first = true;
      for (String item : items) {
        if (first)
          first = false;
        else
          s.append(", ");
        s.append(item);
      }
      return s.toString();
    }
    
  7. Checking the type of an exception thrown by code I own
    Here’s an example from the PHP language documentation:

    <?php
    function inverse($x) {
        if (!$x) {
            throw new Exception('Division by zero.');
        }
        return 1/$x;
    }
    
    try {
        echo inverse(5) . "\n";
        echo inverse(0) . "\n";
    } catch (Exception $e) {
        echo 'Caught exception: ',  $e->getMessage(), "\n";
    }
    
    // Continue execution
    echo "Hello World\n";
    ?>
    
  8. Checking the type of an exception thrown by code I do not own
    We often find code such as this in Rails controllers:

    def delete
      schedule_id = params[:scheduleId]
      begin
        Schedules.delete(schedule_id)
      rescue ActiveRecord::RecordNotFound
        render :json => "record not found"
      rescue ActiveRecord::ActiveRecordError
        # handle other ActiveRecord errors
      rescue # StandardError
        # handle most other errors
      rescue Exception
        # handle everything else
      end
      render :json => "ok"
    end
    

Is this list anything like complete? Can it ever be?

I wonder if a useful next step might be to write down some general strategies for removing each of these kinds of conditonal…?

Advertisements

13 thoughts on “Where do conditionals come from?

  1. Awesome work! Very interested in the outcome of this.

    About numbers 3 and 4: I guess we can have different reasons for checking a parameter. Off the top of my head: checking for nil, checking types (as in is_a? or respond_to? in Ruby), and checking value/state for flow control. Since the strategy for getting rid of those can vary, I wonder if the list should be more specific about the reason the conditional exists.

    • @abernardes: My first draft had separate entries for *checking* parameters versus *validating* them. They do feel different, and I don’t know whether that means they will have different kinds of fix.

  2. I was thinking to a strategy to remove the second kind of conditional, and it actually turned out that such conditional MAY have the reason to exist. One simple solution to prevent the if usage would be to use a method that throws an exception (User.find), so that the conditional could be removed. However we observe that:

    1. This leads us to the 8th kind of “if” (e.g. we have to rescue the exception, but I agree that a framework or some other abstraction might hide this via some nice macros)
    2. It may not be what we want to do.

    There might be, in fact, a possibility that such “if” deserves to exist. The reason for that might be that we don’t want to deal with the user model at all, and perform two totally different tasks inside the same method:

    def omg
    if User.exists?(…)
    User.destroy_all
    else
    User.create(…)
    end
    end

    Now, I admit that the above example is pretty much meaningless, but I hope it explains the concept. We might use the solution provided by Piotr Solnica (in his control couple article) and create various classes that handle the if-branches separately even with the code above. However, for such a simple case, I wouldn’t go with that solution because, even though we may have introduced an abstraction (which MIGHT be useful), we have worsened the look of the code (and its understandability) decreasing the cohesion of the method. Moreover, new features will arrive and the abstraction could be wrong for such new features. Thus, I would wait a bit before transforming that if clauses into an abstraction.

    So, to wrap up, I think that the scenarios are too many (perhaps infinite) to be written into a blog post. Or maybe I am wrong and I have just proposed other scenarios with this comment :)

    What do you think?

    • I think the second kind is probably impossible to eliminate in general, without redesigning all the other code in the world that might call us. Our apps need to be able to ask about inputs they can’t control (“is that form field empty, or valid?” etc), so that kind is just fine. It’s the others that I think are very often redundant…

  3. Pingback: Eliminate many conditionals with this one weird trick | silk and spinach

  4. After reading your posts on this, I decided to check some code I had recently written. I found one main type of if statement. It looks like this:
    if (_input.Length > 0)
    {
    result = ProcessExpression();
    }
    else
    {
    throw new MalformedExpressionException(ParserResources.EmptyInput);
    }
    So – if things are not going as we expect, then throw an exception.
    What would be your suggestion for eliminating this kind of if statement?

    • @Clare — This feels like type 4, validating inputs. Assuming, of course, that an empty string is indeed invalid. In general I think it’s perfectly okay to reject invalid inputs, so we have to have conditionals around the edges of our hexagons.

      So my only question would be: Is there a nice default behaviour we could apply to an empty input string? Such as having the parser simply return empty output?

  5. PS The code in question (as you may have guessed) parses a string (it was actually a TDD exercise, so a little forced, but interesting nonetheless). The only other if statement in my code was this one:

    if (IsPartOfDecimal(NextCharacter))
    {
    nextNumber = ExtractNextDecimal();
    }
    else if (IsOpeningBracket(NextCharacter))
    {
    nextNumber = CalculateExpressionWithinBrackets();
    }
    else
    {
    throw new MalformedExpressionException(string.Format(ParserResources.NumberOrOpeningBracketExpected, NextCharacter, _nextIndex));
    }
    You are in the middle of processing the input, character by character. You know the next character should either represent a number, or be the opening bracket of a sub-expression. It could be either one. Hence the if statement. Unless I’m missing something, it doesn’t fall into the list in your post. I suppose you could count it as living in the hexagonal boundary with the outside world, because we are checking an external input… but that doesn’t seem right either.
    I pondered writing a char extension which contained some intelligence about what to do depending on whether the char was part of a decimal, or an opening bracket… but when I started trying to write it, I hit a lot of complexity very quickly, and was still struggling to avoid the if statement. Should I have persevered, do you think, or is there some other solution?

  6. Having thought about it some more, I suppose the answer could be some sort of ExpressionToken class, which can be initialised with a string and a start index, and would extract the next decimal or bracket or whatever else, and would know how to act in particular circumstances.
    So, you could replace this…
    if (IsPartOfDecimal(NextCharacter))
    {
    nextNumber = ExtractNextDecimal();
    }
    else if (IsOpeningBracket(NextCharacter))
    {
    nextNumber = CalculateExpressionWithinBrackets();
    }
    …with something like this:
    next Number = (new ExpressionToken(_input, _nextIndex)).EvaluateExpression();

  7. @Clare — Again, you are definitely checking inputs, I think. So there has to be something somewhere that compared the next token with a list of valid next tokens, and again that’s fine.

    I suppose you could have a table/hash/dictionary of possible next tokens mapping to their corresponding actions (or token objects), and throw a SyntaxError when the table lookup doesn’t find the token you have? But either way, there has to be some checking here, at the boundary, so conditionals are (usually) inevitable.

  8. Pingback: On paperboys, newsagents and exceptions | silk and spinach

  9. Hi Kevin.
    Thanks for an interesting article!
    You write “I wonder if a useful next step might be to write down some general strategies for removing each of these kinds of conditonal…?”
    To answer your question: Yes, that would be cool. In the article “Eliminate many conditionals with this one weird trick | silk and spinach” you show how to eliminate “Checking a value returned to me from code I own”. It would be great if you could give examples on how to eliminate the other types of conditions.

    Thanks in advance,
    Anders

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s