Optional function parameters: Use default arguments (NULL) or overload the function?


Question

I have a function that processes a given vector, but may also create such a vector itself if it is not given.

I see two design choices for such a case, where a function parameter is optional:

Make it a pointer and make it NULL by default:

void foo(int i, std::vector<int>* optional = NULL) {
  if(optional == NULL){
    optional = new std::vector<int>();
    // fill vector with data
  }
  // process vector
}

Or have two functions with an overloaded name, one of which leaves out the argument:

void foo(int i) {
   std::vector<int> vec;
   // fill vec with data
   foo(i, vec);
}

void foo(int i, const std::vector<int>& optional) {
  // process vector
}

Are there reasons to prefer one solution over the other?

I slightly prefer the second one because I can make the vector a const reference, since it is, when provided, only read, not written. Also, the interface looks cleaner (isn't NULL just a hack?). And the performance difference resulting from the indirect function call is probably optimized away.

Yet, I often see the first solution in code. Are there compelling reasons to prefer it, apart from programmer laziness?

1
41
3/31/2009 11:15:20 PM

Accepted Answer

I would definitely favour the 2nd approach of overloaded methods.

The first approach (optional parameters) blurs the definition of the method as it no longer has a single well-defined purpose. This in turn increases the complexity of the code, making it more difficult for someone not familiar with it to understand it.

With the second approach (overloaded methods), each method has a clear purpose. Each method is well-structured and cohesive. Some additional notes:

  • If there's code which needs to be duplicated into both methods, this can be extracted out into a separate method and each overloaded method could call this external method.
  • I would go a step further and name each method differently to indicate the differences between the methods. This will make the code more self-documenting.
28
4/21/2011 12:23:43 AM

I would not use either approach.

In this context, the purpose of foo() seems to be to process a vector. That is, foo()'s job is to process the vector.

But in the second version of foo(), it is implicitly given a second job: to create the vector. The semantics between foo() version 1 and foo() version 2 are not the same.

Instead of doing this, I would consider having just one foo() function to process a vector, and another function which creates the vector, if you need such a thing.

For example:

void foo(int i, const std::vector<int>& optional) {
  // process vector
}

std::vector<int>* makeVector() {
   return new std::vector<int>;
}

Obviously these functions are trivial, and if all makeVector() needs to do to get it's job done is literally just call new, then there may be no point in having the makeVector() function. But I'm sure that in your actual situation these functions do much more than what is being shown here, and my code above illustrates a fundamental approach to semantic design: give one function one job to do.

The design I have above for the foo() function also illustrates another fundamental approach that I personally use in my code when it comes to designing interfaces -- which includes function signatures, classes, etc. That is this: I believe that a good interface is 1) easy and intuitive to use correctly, and 2) difficult or impossible to use incorrectly . In the case of the foo() function we are implictly saying that, with my design, the vector is required to already exist and be 'ready'. By designing foo() to take a reference instead of a pointer, it is both intuitive that the caller must already have a vector, and they are going to have a hard time passing in something that isn't a ready-to-go vector.


Licensed under: CC-BY-SA with attribution
Not affiliated with: Stack Overflow
Icon