Multiple arguments in function call vs single array

Question

I have a function that takes in a set of parameters, then applies to them as conditions to an SQL query. However, while I favored a single argument array containing the conditions themselves:

function searchQuery($params = array()) {
    foreach($params as $param => $value) {
        switch ($param) {
            case 'name':
                $query->where('name', $value);
                break;
            case 'phone':
                $query->join('phone');
                $query->where('phone', $value);
                break;
        }
    }
}

My colleague preferred listing all the arguments explicitly instead:

function searchQuery($name = '', $phone = '') {
    if ($name) {
        $query->where('name', $value);
    }

    if ($phone) {
        $query->join('phone');
        $query->where('phone', $value);
    }
}

His argument was that by listing the arguments explicitly, the behavior of the function becomes more apparent - as opposed to having to delve into the code to find out what the mysterious argument $param was.

My problem was that this gets very verbose when dealing with a lot of arguments, like 10+. Is there any preferred practice? My worst-case scenario would be seeing something like the following:

searchQuery('', '', '', '', '', '', '', '', '', '', '', '', 'search_query')

Answer 1

IMHO your colleague is correct for the above example. Your preference might be terse, but its also less readable and therefore less maintainable. Ask the question why bother writing the function in the first place, what does your function 'bring to the table'- I have to understand what it does and how it does it, in great detail, just to use it. With his example, even though I am not a PHP programmer, I can see enough detail in the function declaration that I do not have to concern myself with its implementation.

As far as a larger number of arguments, that is normally considered a code smell. Typically the function is trying to do too much? If you do find a real need for a large number of arguments, it is likely they are related in some way and belong together in one or a few structures or classes (maybe even array of related items such as lines in an address). However, passing an unstructured array does nothing to address the code smells.

Answer 2

My answer is more or less language agnostic.

If the only purpose of grouping arguments in a complex data structure (table, record, dictionary, object...) is to pass them as a whole to a function, better avoid it. This adds a useless layer of complexity and makes your intention obscure.

If the grouped arguments have a meaning by themselves, then that layer of complexity helps understand the whole design: name it layer of abstraction instead.

You may find that instead of a dozen individual arguments or one big array, the best design is with two or three arguments each grouping correlated data.

Answer 3

In your case, I would prefer your colleague's method. If you were writing models and I was using your models to develop over them. I see the signature of your colleague's method and can use it immediately.

While, I would have to go through the implementation of your searchQuery function to see what parameters are expected by your function.

I would prefer your approach only in the case when the searchQuery is limited to searching only within a single table, so there will be no joins. In that case my function would look like this:

function searchQuery($params = array()) {
    foreach($params as $param => $value) {
        $query->where($param, $value);
    }
} 

So, I immediately know that the elements of array are actually the column names of a particular table which the class having this method represents in your code.

Answer 4

Do both, sort of. array_merge allows for an explicit list at the top of the function, as your colleague likes, while keeping the parameters from getting unwieldy, as you prefer.

I also strongly suggest using @chiborg's suggestion from the question comments - it's a lot clearer what you intend.

function searchQuery($params = array()) {
    $defaults = array(
        'name' => '',
        'phone' => '',
        ....
    );
    $params = array_merge($defaults, $params);

    if(!empty($params['name'])) {
        $query->where('name', $params['name']);
    }
    if (!empty($params['phone'])) {
        $query->join('phone');
        $query->where('phone', $params['phone']);
    }
    ....
}

Answer 5

Also you could pass a string resembling a query string, and use parse_str (because it seems you are using PHP, but other solutions are probably available in other languages) to process it into an array inside the method:

/**
 * Executes a search in the DB with the constraints specified in the $queryString
 * @var $queryString string The search parameters in a query string format (ie
 *      "foo=abc&bar=hello"
 * @return ResultSet the result set of performing the query
 */
function searchQuery($queryString) {
  $params = parse_str($queryString);
  if (isset($params['name'])) {
    $query->where('name', $params['name']);
  }
  if (isset($params['phone'])) {
    $query->join('phone');
    $query->where('phone', $params['phone']);
  }
  ...

  return ...;
}

and call it like

$result = searchQuery('name=foo&phone=555-123-456');

You can use http_build_query to convert from an associative array to a string (the reverse that parse_str does).