Does this function have too many parameters?

Question

In the end, I got this function. I don't know whether it's normal or not.

function user_registration($user_name, $user_email , $user_pass , $address , 
    $city , $postalcode  , $country  , $phone , $mobilephone)

How and why can I improve this?

Answer 1

A solution would be to only have one parameter, that can contain several pieces of data -- like an array.

Your function could be defined this way :

function user_registration(array $data) {
    // work with $data['name']
    // and $data['email']
    // ...
}

And you'd call it like this :

user_registration(array(
    'name' => 'blah',
    'email' => '[email protected]', 
    'pass' => '123456',
    // and so on
));

Nice things are :

• You can add / remove "parameters" easily
• The "parameters" can be passed in any order you want

Not so bad things are :

• No hint while typing in your IDE
• No documentation (like phpDoc)

Answer 2

One way is to pass an array as parameter to this function and put all info in that array:

function user_registration(array $user_info)
{
   // process $user_info;
}

Answer 3

You could either pass an array with all variables packed nicely together, or just make a "User" class and add all properties via setters and do the validation in the end with a dedicated method:

class User {

  public function setName($name) {
    $this->name = $name;
  }

  [...]

  public function register() {

    //Validate input
    if (empty($this->name))
      $this->errors[] = "ERROR, Username must not be emtpy";

    //Add the user to the database
    //Your SQL query
    return empty($this->errors);
  }

}

$user = new User();
$user->setName("Peter");
$success = $user->register();

if (!$success)
  echo "ERRORS OCCURED: ".print_r($user->errors, true);

Answer 4

I personally don't think it has too many parameters. From looking at the functions definition it is clear what you require as input which wouldn't be so obvious if called with an array.

"If it aint broke don't fix it!"

Answer 5

I'd make it this way

fields=explode(",","name,surname,lastname,street,city,region,zip,country");
user_registration($fields);

Because I am sure these variables coming from $_POST

Answer 6

I make array of keys like

$fields = array('field1', 'field2');
function register (array $values, array $keys)
{
    $data = array();
    foreach ($keys as $one)
    {
        if (isset($values[$one])) $data[$one] = $values[$one];
    }
    // or you can use array functions like array_flip and after - array intersect
}

Answer 7

Function (method) without any parameters is best. Function with one paremeter is better than function with 2 parameters. Function with 2 parameters is better than function with 3 parameters and so on.

Answer 8

As a general rule of thumb (not as a steadfast rule), anytime you have to ask "Does this function have too many parameters?" -- the answer is yes. Your intuition is telling you something that your brain just hasn't yet been able to work out.

In this particular case, the first thing that comes to mind is that your user cred.s should be checked first (does the username already exist? is the pw complex enough) and your user details should be added separately, probably using an object or array.

Answer 9

@Florianh has given a perfect solution how your code can be improved. With this comment, I would like to elaborate on the "why" part from a system design perspective.

From an Object Oriented perspective, other objects should be able to manipulate attributes. That is why attributes should never be defined as "public". They should be "private" e.g.:

private var $name;

The reason for this is when other objects would manipulate this variable, the correct working of the object is at risk. Methods, on the other hand, can be defined publicly:

public function register() 

Accordingly, the manipulation of the attributes will happen through the appropriate methods. A method can be used to also evaluate the correctness of operations on the attributes.

There are two operations that can happen: reading the current value of an attribute by using Get methods and saving a new value of an attribute by using Set methods.

A good practice would be to implement a get method for each class attribute. Each attribute that can be modified, should have an appropriate set method as well.

Sometimes it is better to not implement a get/set method (e.g.: showData()) after all. This is because the usage of getters and setters within a particular class may possibly lead to a reduced performance. However, this means that when changing or implementing the class, one must be careful when trying to save false information and as a result putting the integrity of the class at risk.

Now, consider the fact that you decide to use only one phone number instead of both a phone- and mobile number. When the mobile number becomes deprecated, your main program still remains the same. All you have to do is change/remove one method. The advantage of using getters and setters is an increase in adaptability and maintainability.