?>

The argument for flag arguments

I recently read “Clean Code” by Robert Martin an excellent book on writing clear, easy to maintain and well factored code. In it Robert Martin raises the point that methods should do what their names say, shouldn’t have ‘flag arguments’, and should do only one thing. This implies that overloaded methods are out. While I think naming methods after what they do is an excellent idea, I’m not totally sold on the ‘no boolean arguments’ part. Robert Martin suggests that instead of a single method with a boolean argument you should refactor that into 2 public methods and one or more protected methods. Take for example CakePHP’s Model::bindModel():

Show Plain Text
  1. $this->Post->bindModel(array('hasMany' => array('Comment')));
  2. $this->Post->bindModel(array('hasManyAndBelongsToMany' => array('Tag')), false);

While these methods calls look strikingly similar they behave very differently. One creates a temporary association that lasts for only one find call, the other creates a permanent association. But determining which is which is the hard part, and the source of Robert Martin’s rule of ‘no boolean arguments’. Instead Robert Martin suggests that you use separate public methods for each function the flag operator makes.

Show Plain Text
  1. $this->Post->bindModelTemporary(array('hasMany' => array('Comment')));
  2. $this->Post->bindModelPermanent(array('hasManyAndBelongsToMany' => array('Tag')));

While the lack of flag arguments makes the code considerably easier to read and understand to the new developer. I don’t know if its necessarily better in the long run. Some drawbacks to this approach are you end up with a larger public and internal API, as more methods are needed to accomplish the same tasks. Larger API’s in my experience take more brainpower to remember. While its true that modern day code completion in IDE’s can minimize this drawback. However, when working with dynamic languages like Javascript, PHP and Python IDE autocomplete support can be pretty dismal at times. I personally find 10 small methods with infrequently used alternate operators are easier to remember, than 20 discrete methods. Not using flag arguments also enter the realm of overloaded method signatures. I don’t think overloaded method signatures are to be avoided though. A hugely popular example of method overloading can be found in jQuery.

Show Plain Text
  1. $('#thing').attr('name'); //getter
  2. $('#thing').attr('name', 'John'); //setter
  3. $('#thing').attr({name: 'John', rel: 'father'});

The attr method is both a getter and a setter. Depending on the type and number of arguments passed it can behave in one of many ways. One method doing many jobs depending on what you provide it. jQuery has become a successful and well adopted library partly due to its small, and easy to remember feature set. So would the following be an improvement, or just unnecessary verbosity?

Show Plain Text
  1. $('#thing').getAttribute('name');
  2. $('#thing').setAttribute('name', 'John');
  3. $('#thing').setAttributes({name: 'John', rel: 'father'});

The second example while clearer to the casual reader, is just more work to those familiar with jQuery. Perhaps flag arguments and heavily overloaded methods are difficult to use without context and experience with the codebase being worked on, but separate methods can be cumbersome to the experienced user. I think discretion is important when considering whether or not to use separate methods or used overloaded methods and flag arguments. In either case I don’t think boolean arguments are going to leave my bag of tricks any time soon. And think that the occasional flag argument can actually simplify a code base.

Comments

From what I’ve read, using flag arguments as-is passing raw true/false is not good practice for the exact reason you state: the developer has no clue what that argument does by just looking at it. You’re stuck referencing the documentation or your code completion.

The approach I think makes most sense is using named class constants with integer values to provide true/false values and beyond. Flag arguments limit you to 2 options: true and false. What happens when you need to expand your method to handle 3 options?

Doctrine ORM 1.x uses this approach. Check out the Doctrine_Core class and the relevant documentation: http://www.doctrine-project.org/documentation/manual/1_2/en/configuration

John Kary on 1/14/10

I think we’re talking about two different things here.

  • “No boolean flag arguments”: agree on this one. If you read the code, you don’t understand it. And it’s difficult to remember which one is the default. By the way, CakePHP’s bind() default parameter is permanent = true, while bindModel() default parameter is reset = true. Very hard to remember, even in methods written by myself.
  • “No overloading”. I don’t agree. jQuery’s attr method is easy to read and easy to remember.

Javi on 1/15/10

Maybe you can get some referral fees on the book you mention. I’m am searching for it now ;)

primeminister on 1/15/10

The problem consists in limitations of diferent languages. In PHP implement methods overloading will looks not very good.
This mean that here better to use boolean flags, or diferent methods names.
But in strict typed languages way of overloaded methods most time is good till developer did not start to create 20 diferent methods versions like it done in .Net.

skiedr on 1/15/10

I believe Javi has a point here.

I would argue that jQuery’s approach with $(element).attr() simply feels natural when you use it.
As compared to

$this->Post->bindModel(array(‘hasManyAndBelongsToMany’ => array(‘Tag’)), false);

where you have no idea what “false” triggers.
I like hashes in Ruby, passing named arguments like {:name => ‘foo’} to a method speaks for itself.

After all, though a bit cumbersome, array(‘conditions’ => array()) etc. does the job, too.

leo on 1/15/10

John Kary: I think the class constant approach used by Doctrine provides a more readable twist on flag arguments. Boolean flags are by far the most limited of flags, and named integer flags while more flexible and more readable are still ‘flags’ at their core.

Javi: Model::bind() is being removed in CakePHP 1.3, as having two methods that do the same thing, is also a ‘bad’ thing in my books. I didn’t mean to imply that overloaded methods were bad, in fact I think they are a very important part of dynamic languages. I was more referring to the idea that each method should do what it says. A method that that can do 3 different things is probably not following that rule. So it was more a questioning of the suggestions provided in the book.

leo: Would Model::bindModel() be more palatable if written like $Post->bindModel(array('belongsTo' => array('Author'), Model::BIND_PERMANENT); ? I guess what I’m trying to get at, is the concept of ‘flags’ really flawed, or is it more floaty booleans that cause issues.

mark story on 1/15/10

In the case of passing true boolean flags I find the most useful scenario is one where the return value of a function is used as opposed to a pure boolean representation of true or false.

I.E. I find it much easier to read code that looks like…

$Post->bindModel( array( ‘belongsTo’ => array( ‘Author’ ),
), $Post->booleanFunction( ));

Where the $Post->booleanFunction( ) returns a true or false depending on whether the bind is temporary or not.

Without boolean flags you would have to use two different functions who’s only difference is negligible and as a result would have to remember a number of different functions to achieve the same result.

Abba Bryant on 1/15/10

Ruby has the nice ! which marks a method to be “modifing” data.

Foo = 5
puts Foo.toString # ‘5’
puts Foo # 5
Foo.ToString!
puts Foo.ToString # ‘5’

I like that way of having getter and setter, by a short and small symbol – I might be wrong though.

someone on 1/23/10

Python named arguments solve htis problem!

Matt on 2/8/10

I think Boolean arguments aren’t a good idea because like others were saying you can’t really remember what they are or where they go, then you need to look up the documentation.

I typically just have one big array with words I can remember like:
$class->function(array(‘feature_x’ => TRUE, ‘feature_y’ => FALSE));
easier than
$class->function(FALSE, TRUE);

Sean on 3/16/10

Comments are not open at this time.