Code refactoring: third party libraries

I am trying to refactor my code to make it more decoupled. Specifically I have a lot of areas where I’m using third party libraries to interact with an external service.

For example, here is the code I use to authorize a transaction through our gateway:

$gw = new Products_GatewayAPI();
$gw->setLogin('username', 'password');
$gw->setBilling($_POST['firstname'],$_POST['lastname'],$_POST['company'],$_POST['address1'],$_POST['address2'],$_POST['city'],$_POST['state'],$_POST['zip'],$_POST['country'],$_POST['phone'],$_POST['fax'],$_POST['email'], $_POST['website']);
$gw->setShipping($_POST['shipping']['firstname'],$_POST['shipping']['lastname'],$_POST['shipping']['company'],$_POST['shipping']['address1'],$_POST['shipping']['address2'], $_POST['shipping']['city'],$_POST['shipping']['zip'],$_POST['shipping']['state'],$_POST['shipping']['phone'],$_POST['shipping']['fax'],$_POST['shipping']['email'], $_POST['shipping']['website']);
$gw->setOrder($this->order_id, "Standard Website Order",$this->registry->cart->getTax(),$this->registry->cart->getShipping(),null,$_SERVER['REMOTE_ADDR']);
$response = $gw->doAuth($this->registry->cart->getTotal(),$_POST['ccnumber'],$_POST['ccexp'], $_POST['cvv']);

As you can see, my code is dependant on this library’s interface.

The first thing I’d like to do in refactoring this code is create an Order object that contains two Address objects: billing and shipping and a CreditCard object.

The above code then becomes a sort of translation from my api (order object) to their api (transaction-style methods).

The next thing I would like to do would be to wrap the above code in a class, used like this:

$translator = new Translator_NMI($order); // I probably wouldn't call this class Translator, but it works for this example.
$response = $translator->authorize();

My code is now dependent on my translator interface, which accepts an order object and potentially returns a response object with detailed information about the transaction.

This is my best guess for how to solve this particular problem. How would you?

In the first case, I absolutely agree with building your own custom interface to wrap the external dependencies. That gives you good change management. If the next version out of the library is a major rewrite and important things change, you only need to rewrite your custom interface / wrapper or whatever you want to call it.

Ahhhh… this is an interesting question and I agree with kyberfabrikken’s solution. What you have to understand here is that dependence is a double edged sword. Dependence gets a really bad rap, and rightly so; the more dependent your code is, the more inflexible it becomes and this makes modification a nightmare (especially when changes force you to search your entire code base to make updates). However, dependence is also how you get things done using well abstracted utility functions and classes. The secret is not to eliminate dependence in your code. You want to depend on the fewest number of things necessary to get the job done, and depend on absolutely nothing else. This includes rewriting code, reorganizing classes, and otherwise reworking your design if it allows you to more effectively reduce the amount of dependence in your functions.

That’s why the interface is such a great choice in this, and many other scenarios; the interface defines exactly what needs to occur, no more no less (obviously most relevant here is that there’s no more because that would add dependence). Then you write functions against the interface and you can pass any concrete class which implements that interface to those functions. It’s a fantastic way to reduce dependence to the absolute minimum.

Sounds about right, to me anyway. Although, I’d probably look at implementing an Interface for the Processor and a ServiceLocator for it.

This would allow the client code to be blissfully unaware what Processor you’re using.


interface PaymentProcessor
{
  public function process(Customer $customer, Order $order);
}


class MockPaymentProcessor implements PaymentProcessor
{
  public function process(Customer $customer, Order $order){
    $response = new PaymentProcessorResult();
    return $response->setStatus(PaymentProcessorResult::STATUS_ACCEPTED);
  }
}


class PaymentProcessorResult
{
  const
    STATUS_DENIED     = 1,
    STATUS_ACCEPTED   = 2,
    STATUS_CANCELLED  = 3,
    STATUS_PENDING    = 4,
    STATUS_UNKNOWN    = 5;
    
  protected
    $status = null;
    
  public function setStatus($status){
    $this->status = $status;
    return $this;
  }
  
  public function getStatus(){
    return $this->status;
  }
}

This allows you to create an unlimited amount of payment processors without changing any client/current code.


class PaypalPaymentProcessor implements PaymentProcessor
{
  public function process(Customer $customer, Order $order){
    
  }
}

class GooglePaymentProcessor implements PaymentProcessor
{
  public function process(Customer $customer, Order $order){
    
  }
}

class SagePayPaymentProcessor implements PaymentProcessor
{
  public function process(Customer $customer, Order $order){
    
  }
}

Pop a service locator on top of this, and I’d say you have a pretty flexible solution.

Well, that’s orthogonal to my point, but I also suggest that Mailer is a wrapper around the external library.

If you inject Mailer into your application code, then that code is only dependent on something with a “send” interface.

In this case, is Mailer the external library’s api or my wrapper over it? If the former, it still seems like I’m dependent on the send() method.


<?php
class SagePayPaymentProcessor implements PaymentProcessor
{
    protected
        $sagepay = null;
        
    public function __construct(SagePay $sagepay){
        $this->sagepay = $sagepay;
    }
    
    public function process(Customer $customer, Order $order){
        #do stuff, then a little more stuff.
        $result = $this->sagepay->proprietary(array());
        return true; #maybe, or an object - not an orange. Definitely not fruit.
    }
}
?>

Two thing that I find useful when dealing with external libraries, is 1) don’t use the api directly, less you want to tie your self to a single provider. wrap it in your own interface, that suits your applications needs. 2) separate state and side effects.

Eg. instead of something like this:


$mail = new Mailer();
$mail->subject = "foo";
$mail->send();

do something like this:


$mailer = new Mailer();
$mail = new Mail();
$mail->subject = "foo";
$mailer->send($mail);

This allows you to inject the gateway (Mailer) into your code easily, without having to change any application code.