Default Parameters

As of today, I no longer like default parameters. Three times in the past week I have been caught out by default parameters, some of it in my own code! You would think I would know what is going on.

I like default parameters, I think they are are good way of allowing optional parameters on interfaces and they can provide an automatic suite of function overloads. However, when it comes to refactoring and debugging they can be hazardous.

The first scenario was when I was refactoring some collection code to accept a "catalogue" type that would define what is allowed in the collection. This code wasn't my own but I had designed the overall solution. Anyway every parameter on the collection constructor had a default value (one may argue, why have parameters at all, but there was a fair bit going on behind the scenes). I removed some no redundant parameters and added my catalogue. No compiler errors but the code didn't work. I had made changes to about 3 classes for this change, so it took a little while to work out what I had done wrong.

I had forgotten to take a parameter out of some calling code, and because the defaults were optional, this parameter was pushed down the list. Hence no compiler error, and technically a valid object, it just wasn't what my code was expecting. A while later we're in Crash-ville. Naturally I should have (and this was how I resolved it) found the references to the constructor and made sure they were correct. This was only a right-click away but I had gotten lazy on compiler warnings.

The second scenario was a bit of a doozy. It relates to the same set of objects. The catalogue has a function that when an object is passed to it, it returns true if the object is a match for the catalogue, false if it isn't. There is a default parameter that specifies whether the object passed in should update some information inside the catalogue. I only needed the update in a few places (less than I don't need it), so the default is false (or don't update).

my code reads something like this:

if (collection->getCatalogue()->isMatch
                 ((*objectItr)->m_element))
{
 // we have our match, catalogue is unchanged
}

//OR

if (collection->getCatalogue()->isMatch
                 ((*objectItr)->m_element), true)
{
 // we have a match and our catalogue was updated
}

Apologies for the formatting. I need to resize this blog, it's like trying to write on the side of a milk carton. Just imagine the if-statement all on one line.

Now, that code compiles and works fine for a number of test scenarios. However, it always evaluates to 'true'. The reason is, the true at the end of the if-statement should be a parameter inside isMatch, but it isn't so isMatch defaults (giving me incorrect logic) and the true is evaluated by the comma-operator. Fun-times.

The correct if-statement is:

if (collection->getCatalogue()->isMatch
                 ((*objectItr)->m_element, true))
{
 // we have a match and our catalogue was updated
}

With the context removed my original code is resembling something similar to this:

if (condition, true)
{
 //always evaluate
}

In reality I should have named my methods better (Yes, I know, I harp on about test-case naming conventions and then do this).

• isMatch (object)
  
• isMatchWithCatalogueUpdate (object)

or potentially

• isMatch(object)
  
• UpdateCatalogue (object) --called within the if-statement

I no longer have default parameters and I do have code that works. I suspect my usage of default parameters will approach zero from here on in. I won't discount them entirely as that is foolish, I'll just think twice before using them.