PHP Functions – Multiple Arguments vs Single Array

functionsparametersPHPswitch statement

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')

Best Answer

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.

Related Solutions

Design – Does designing a method that changes the argument’s (if was object) values a good practice

If you have method like that, then such method should actually be part of Player class. This then clearly indicates the method might change the state of the object.

class Player
{
    void AddLevelBy1()
    {
        this.level += 1;
    }
}

player.AddLevelBy1()

The problems start when you have more than 1 objects being changed, but there is no general solution to that.

If using c#, the "out" keyword can be used to indicate the change.

This is wrong. out keyword is to indicate something completely else. See more here: https://stackoverflow.com/questions/9875333/c-sharp-passing-reference-type-directly-vs-out-parameter

Design Patterns – Best Design Pattern for Library with Data Model

I could be waaaaaay off, but...

It seems that a model is necessary to use the library, you just don't care which one, right? Further, the library doesn't care how the model gets its data, or where the data comes from, it just wants an object that meets the criteria of the interface.

If the library doesn't need the connection object, then make that the responsibility of the model.

interface MyFooModelInterface 
{
    public function getItem($id);
    public function addItem($item);
    ...
}

And here is a simple class example:

class MyLib
{
    protected $model;

    public function __construct(MyFooModelInterface $model)
    {
        $this->model = $model;
    }

    public function getItemById($id)
    {
        return $this->model->getItem($id);
    }
}

And a Model example:

class MyModel implements MyFooModelInterface
{
    protected $conn;

    public function __construct(\PDO $conn)
    {
         $this->conn = $conn;
    }

    public function getItem($id)
    {
        ...
    }
    public function addItem($item)
    {
        ...
    }
}

And here is an example of using it:

// the model is responsible for handling the data retrieval, not the library
$model = new Model($conn);

$lib = new MyLib($model);
return $lib->getItem($id);

Now, if I decide that I want a file-based solution, I can easily swap the model. To test the model, the connection obj is injected, so that's easy. To test the library, the model is also injected, so, again, that's easy.

In this case, the model is only responsible for getting the data, while the library is responsible for handling and processing the data. Neither cares what or how the other one does it.

EDIT

the library with a factory method to create the model if none was given...

class MyLib
{
    protected $model;

    public function __construct(MyFooModelInterface $model = null, $conn = null)
    {
        // if we have a model, use it
        if (!is_null($model)) { 
            $this->model = $model; 
        }

        // if we don't have a model, but have a connection obj, make a model
        if (is_null($model) && !is_null($conn)) { 
            $this->model = $this->factoryModel($conn); 
        }

        // if we don't have either, quit and go home
        if (is_null($model) && is_null($conn)) {
            throw new Exception('need moar inputs');
        }
    }

    protected factoryModel($conn)
    {
        $this->model = new MyModel($conn);
    }

    public function getItemById($id)
    {
        return $this->model->getItem($id);
    }
}

Best Answer

Related Solutions

Design – Does designing a method that changes the argument’s (if was object) values a good practice

Design Patterns – Best Design Pattern for Library with Data Model

EDIT

Related Topic