# Payment function with too many parameters

I have a method with too many parameters.

How would you refactor it?

Things I considered so far:

• pass an array to the object and check for required keys (no code hint, phpdoc, programmer doesn't know which are required)
• pass an object (silly method with getters and setters, programmer still doesn't know which are required)

Here's the method:

public function doDirectPayment(
$amount,$credit_card_type,
$credit_card_number,$expiration_month,
$expiration_year,$cvv2,
$first_name,$last_name,
$address1,$address2,
$city,$state,
$zip,$country,
$currency_code,$ip_address,
$payment_action = 'Sale' ) {$client = $this->getClient();$client->setParameterGet('METHOD', 'DoDirectPayment');

$month = str_pad(ltrim((string)$expiration_month, 0), 2, '0', STR_PAD_LEFT);
$expiration_date =$month . $expiration_year;$client->setParameterGet('PAYMENTACTION', urlencode($payment_action));$client->setParameterGet('AMT', urlencode($amount));$client->setParameterGet('CREDITCARDTYPE', urlencode($credit_card_type));$client->setParameterGet('ACCT', urlencode($credit_card_number));$client->setParameterGet('EXPDATE', urlencode($expiration_date));$client->setParameterGet('CVV2', urlencode($cvv2));$client->setParameterGet('FIRSTNAME', urlencode($first_name));$client->setParameterGet('LASTNAME', urlencode($last_name));$client->setParameterGet('STREET', urlencode($address1)); if (!empty($address2)) {
$client->setParameterGet('STREET2', urlencode($address2));
}

$client->setParameterGet('CITY', urlencode($city));
$client->setParameterGet('STATE', urlencode($state));
$client->setParameterGet('ZIP', urlencode($zip));
$client->setParameterGet('COUNTRYCODE', urlencode($country));
$client->setParameterGet('CURRENCYCODE', urlencode($currency_code));
$client->setParameterGet('IPADDRESS', urlencode($ip_address));

$response =$client->request(Zend_Http_Client::GET);

return $response; }  • biggest argument list I've ever seen :) – webarto Jul 29, 2011 at 21:52 ## 3 Answers I'd group the parameters: doDirectPayment(CreditCard$card, User $user, array$options = array())


But this doesn't solve all problems.

To mark something required you'd use interface, but the interface can only force the existence of the methods, not the properties.

So the best option seems to be forcing the params in the constructor:

class CreditCard {
public function __construct($required1,$required2 /*, etc*/);
}


But this way you can't instantiate the object without providing options (no fluent interface).

You may mark the params optional and provide some validation method. The problem is, how to automatically call this method. Maybe some SPL interface may help here.

From Fowler's Refactoring:

Long Parameter Lists

Too many parameters. Functionality is wrong, or not enough use of fields

Main Refactoring: Replace Parameter with Method, Preserve Whole Object, Introduce Parameter Object

Looks like most of those params will come from the user filled form, so you could simply do:

doDirectPayment(PaymentForm $form, array$otherOptions = array()) {
// ...
foreach ($form->getValues() as$name => $value) {$client->setParameterGet(strtoupper($name), urlencode($value));
}
// ...
}

• +1 the constructor of the data object that will be the parameter causes required fields, then privatizing the setters on those ensures relative immutability so they can't be replaced with nulls. This is exactly the correct approach from my experience. Jul 30, 2011 at 23:48

Passing an object (class name 'Payment' perhaps?) is not that silly actually. You can add public method validate() to it, that could be called from doDirectPayment() in order to check if all required fields have been filled.

You will probably want to make it as value object so as to avoid unexpected side effects.

• also, if you use an object, return \$this from the setters so function calls can be chained. But the array solution is equally fine, I guess it's a matter of taste..
– yi_H
Jul 29, 2011 at 20:30

I'm not entirely sure if this is feasible in PHP, but a class-seperated builder pattern is a nice way to alleviate long parameter lists.

You can have each builder return the next component as required.

ie:

CreditCardInfo.Build(credit_card_type, _number, month, year) // Returns NameInfo Builder class
.Build(first_name, last_name) //Returns AddressInfo Builder class