Refactoring to interfaces

The information below was delivered to one of my programmers as direction for how to implement a rather big change in an existing software product that I sell. I thought it was potentially useful to a broader audience, so I’m posting it here:

…The rest of this is rather complicated to explain online. I’ll do my best. I’m going to look at this in a simplistic way and let you work through the details.

First imagine that we have an Authorize.net processing class based largely on their API.

class AuthnetProcessAIMPayment {
	protected $apiKey;
	protected $transactionKey;
	protected $x_first_name;
	protected $cardNumber;
	protected $expDate;
	protected $ccv;
 
	function __construct($apiKey, $transactionKey) {
		// initialize object instance
	}
 
	// getter and setter functions
	function setX_first_name ($x_first_name) {
		$this->x_first_name = $x_first_name;
	}
	function getX_first_name () {
		return $this->x_first_name;
	}
	...
 
	function execute () {
		// process payment here
	}
}

I could use this in a script like authnet_process.php

// get $myApiKey and $myTransactionKey values
$payment = new AuthnetProcessAIMPayment($myApiKey, $myTransactionKey);
$payment->setX_first_name("Daniel");
...
$payment->execute();

Now there’s a problem with this approach when it comes to extending our software. You’re about to create an integration with payflowpro. Let’s imaging that the payflowpro class looks like this.

classPayPalPayflowProPayment {
	protected $securityToken;
	protected $paypalEmailAddress;
	protected $name_first;
	protected $cardNumber;
	protected $expDate;
	protected $ccv;
 
	function __construct($securityToken, $paypalEmailAddress) {
		// initialize object instance
	}
 
	// getter and setter functions
	function setName_first ($name_first) {
		$this->name_first = $name_first;
	}
	function getName_first () {
		return $this->name_first;
	}
	...
 
	function execute () {
		// process payment here
	}
}

Here is what you’ll be tempted to do in authnet_process.php (but it’s a mistake)

$paymentProcessor = get_option("authnet_payment_processor");
if ($paymentProcessor == "authnet") {
	// get $myApiKey and $myTransactionKey values
	$payment = new AuthnetProcessAIMPayment($myApiKey, $myTransactionKey);
	$payment->setX_first_name("Daniel");
	...
	$payment->execute();
} else if ($paymentProcessor == "paypal") {
	// get $mySecurityToken and $myPaypalEmailAddress values
	$payment = new classPayPalPayflowProPayment($mySecurityToken, $myPaypalEmailAddress);
	$payment->setName_first("Daniel");
	...
	$payment->execute();
} else {
	// error out, invalid payment processor
}

There are a few problems with this. First is that you now have several lines of code that are almost duplicate to set name, cardNumber, etc. The other is that for each new payment processor (like bluepay coming up) you now add more conditionals. Conditionals increase the chances of bugs and they make the code less clear. Another problem is that the names for common values are likely to be different between implementations. For example, authnet may call it “x_first_name” and paypal might call it “name_first”. Even though you’re providing the same value to it, each payment processor class receives it a different way. This divergence can make it difficult to identify bugs and know which token to search for when you’re making changes.

The ideal that we’re looking for in this case is a solution that will allow authnet_process.php to remain the same regardless of how many payment processors we implement. In answer to one of your questions, we also want our settings interface to be as insulated as possible. In other words, we don’t want to have a different settings mechanism for each payment processor. In even other words, we want each payment processor to accommodate as many of the settings that we’ve already implemented, so that the user has the most seamless experience possible.

You can think of this as being similar to your autoresponder integration in subscription mate. That was done very well. and it didn’t bleed into other settings. You could change the autoresponder provider without requiring changes anywhere else.

So, how do we do this?

Wouldn’t it be great if we could just give a different processing object to authnet_process.php and it knew what to do with it? That’s exactly what we can do with interfaces. Dependency Injection (DI) can be a huge benefit too, but PHP isn’t wired for DI like other languaes, so let’s start with the interface.

An interface cannot be instantiated. Instead, all it does is say that anyone implementing the interface must provide certain functions. Mind you it doesn’t dictate how those functions should be implemented, but you must provide an implementation, even if it does nothing (e.g. a function that just adds a log entry or simply returns an empty value).

The mapping that you were tempted to do in the conditionals above, meaning mapping the clients first name on to one member function for authnet and another for payflowpro, should instead happen one time in the interface implementation.

Here’s what an interface for the above case might look like.

interface OneTimePayment {
	public function setName($name);
	public function getName();
	...
	public function execute();
}

The payment processing classes above would now look like this (I’m showing only the lines that would change).

class AuthnetProcessAIMPayment implements OneTimePayment {
	...
	// getter and setter functions
	function setName ($x_first_name) {
		$this->x_first_name = $x_first_name;
	}
	function getName () {
		return $this->x_first_name;
	}
	...
}

and for payflow pro

class PayPalPayflowProPayment implements OneTimePayment {
	...
	// getter and setter functions
	function setName ($name_first) {
		$this->name_first = $name_first;
	}
	function getName () {
		return $this->name_first;
	}
	...
}

Now that the method names to set and get a name are identical, the code within the conditionals really is duplicate. authnet_process.php can now be simplified to this:

$paymentProcessor = get_option("authnet_payment_processor");
if ($paymentProcessor == "authnet") {
	// get $myApiKey and $myTransactionKey values
	$payment = new AuthnetProcessAIMPayment($myApiKey, $myTransactionKey);
} else if ($paymentProcessor == "paypal") {
	// get $mySecurityToken and $myPaypalEmailAddress values
	$payment = new classPayPalPayflowProPayment($mySecurityToken, $myPaypalEmailAddress);
} else {
	// error out, invalid payment processor
}
// once you create the object, everything after this 
// point can assume it's dealing with a OneTimePayment
// insead of a specific payment processor
$payment->setName("Daniel");
...
$payment->execute();

At this point, even if that conditional grows to accommodate additional payment processors, each one adds only a handful of lines of code to get authentication values and create the object. None of the work to set values and execute transactions will ever need to change, since it treats everything like a OneTimePayment.

How could DI make this even better?

I haven’t seen any good DI frameworks for PHP. In some ways it goes a bit against the grain for PHP development. If there were, this might be how authnet_process.php would look.

$payment = diFramework.get(OneTimePayment);
$payment->setName("Daniel");
...
$payment->execute();

Elsewhere in the DI framework you would define what a concrete instance of OneTimePayment should be. In other words, something like this might be in an XML file (Spring like) or a module (Guice like).

bind(OneTimePayment).to(AuthnetProcessAIMPayment);

Now, anytime you ask for a OneTimePayment, you’ll get an AuthnetProcessAIMPayment object. Similar injection can be used to provide the authentication values, so that in authnet_process.php you really only ask for a OneTimePayment and you get back a functional, ready to use object.

Refactoring

Refactoring is a key aspect of developing software. In this case, where we started with code that creates instances of Authnet classes and uses them directly, changes to authnet_process.php will be necessary. Changes will also be necessary to the Authnet classes to conform with the new interface that we come up with. That may sound like a lot of work.

It is a lot of work, and may actually be more work than just creating a payflowpro class and adding the conditional that I showed at the top. However, there are some concrete benefits and gains we get from doing this additional work.

First is that once the work is done and authnet_process.php uses the interface based calls, all future changes for new payment processors will be very small and won’t functionally endanger any working code for other payment processors.

Second is that there’s a clear scaffolding for adding new payment processors. You can still add as many private internal functions as are necessary and desired to create an “execute” function, but you know that you need one and when it’s done it will work anywhere in your app that you call execute.

Third unittests can more easily test new payment processors without duplicating a lot of code. You simply provide a different OneTimePayment implementation and run the same test. Test coverage stays high and maintenance remains low.

Where do you start?

The best place to start is by coming up with the interface. You want to look at all the values that the current implementation needs and ask if it can be generalized. For example, “x_first_name” might prompt you to design for “first_name” in your interface. A quick check with the documentation for the other payment processor(s) will help you arrive at a comprehensive interface.

Next, make the changes to the Authnet classes so that they implement the new interface(s). Finally work with your code until the unittests pass.

Now you’re ready to implement the interface for payflowpro. As soon as you have done that, you run your unittests and provide an instance of your payflowpro OneTimePayment object.

Once you’re done there and have passing unittests, move on to authnet_process.php.

At some point you’ll need to modify the settings page so that you can provide payflowpro credentials rather than authnet. I really like how you did multiple autoresponder vendors in subscription mate, so I would suggest that as a possible approach for this.

Feel free to create a branch and be daring. You have the security of unittests and the isolation of a branch in svn. If you end up with a mess the first time through, you can start over. I personally think you’ll do great.