Avoid boolean parameters

Here's an example of a method in a sim. This started out as a simple method: all days are the same in our simulation, so the method just prints out some activities:

  void SimulateDay()
  {
    Console.WriteLine("Wake up.");
    Console.WriteLine("Eat breakfast.");
    Console.WriteLine("Watch TV.");
    Console.WriteLine("Eat lunch.");
    Console.WriteLine("Watch TV.");
    Console.WriteLine("Eat dinner.");
    Console.WriteLine("Go to sleep.");
  }

Ok, so not much of a life, but this article is supposed to be about programming. :)

A new requirement comes up: Sunday should be different. Well, almost; it's actually the same as the other days with a single difference: instead of watching TV, the sim goes to church on Sunday mornings. No big deal, we can augment the method with a boolean parameter:

  void SimulateDay(int isSunday)
  {
    Console.WriteLine("Wake up.");
    Console.WriteLine("Eat breakfast.");
    if (isSunday)
      Console.WriteLine("Go to church.");
    else
      Console.WriteLine("Watch TV.");
    Console.WriteLine("Eat lunch.");
    Console.WriteLine("Watch TV.");
    Console.WriteLine("Eat dinner.");
    Console.WriteLine("Go to sleep.");
  }

This works, no doubt about it, but it's a bad idea because the reasons to change this method are different. The sim might get a job; that will affect the work days but not the Sunday. He might become an atheist and stop going to church on Sundays. The two cases are conceptually different.

That means that, for the sake of maintainability, it pays to use two different methods:

  void SimulateWeekDay()
  {
    Console.WriteLine("Wake up.");
    Console.WriteLine("Eat breakfast.");
    Console.WriteLine("Watch TV.");
    Console.WriteLine("Eat lunch.");
    Console.WriteLine("Watch TV.");
    Console.WriteLine("Eat dinner.");
    Console.WriteLine("Go to sleep.");
  }
  
  void SimulateSunday()
  {
    Console.WriteLine("Wake up.");
    Console.WriteLine("Eat breakfast.");
    Console.WriteLine("Go to church.");
    Console.WriteLine("Eat lunch.");
    Console.WriteLine("Watch TV.");
    Console.WriteLine("Eat dinner.");
    Console.WriteLine("Go to sleep.");
  }

and decide at the time of the call which one to use. (I will admit that this contradicts the "defer decisions" principle, but I believe that one is more about design and not about actual ifs in the implementation.)

Of course, the methods as they are now have a lot of duplicate code; those can be extracted into common methods, as long as it makes sense; for example:

  void SimulateWeekDay()
  {
    Morning();
    Console.WriteLine("Watch TV.");
    Afternoon();
    Evening();
  }
  
  void SimulateSunday()
  {
    Morning();
    Console.WriteLine("Go to church.");
    Afternoon();
    Evening();
  }

  void Morning()
  {
    Console.WriteLine("Wake up.");
    Console.WriteLine("Eat breakfast.");
  }
  
  void Afternoon()
  {
    Console.WriteLine("Eat lunch.");
    Console.WriteLine("Watch TV.");
  }
  
  void Evening()
  {
    Console.WriteLine("Eat dinner.");
    Console.WriteLine("Go to sleep.");
  }

(I realize that refactoring some dumb example is really OCD. Hey, I really hate code duplication, ok?)

For the same reason, I would prefer to avoid a method like DoStuff(..., bool withLogging) and just pass a Logger to it instead. Of course, that won't gain me much if instead of peppering the method with if (withLogging) statements I replace them with if (logger != null); a NullLogger that just doesn't do anything would be much better in this case. (Care must be taken so that the expressions sent to the logger are side-effect free, of course, but that's a good idea in itself.)

I've been doing this for a while and the only counter-example I've been able to find is when the booleans are passed on to something else. For example, earlier today I wrote code like this:

  var ck = (bool) item.Value;
  SetEnabled("items", ck);
  SetEnabled("sqlquery", !ck);

where the SetReadOnlyState method sets the Enabled property on a field based on whether a checkbox is ticked or not.

Second rant over. Time to work some more.

Comments

Popular posts from this blog

Posting dynamic Master / Detail forms with Knockout

Comparing Excel files, take two

EF Code First: seeding with foreign keys