This is a prime candidate for the Strategy pattern.
For example, this code:
if ($this->isDisputeAccepted($order)) { //returns true if dispute was accepted
$order->setStatus('accepted');
$order->refund(); //refunds the order on ebay and internally in my system
$this->insertRecordInOrderHistoryTable($order,'refunded');
} else if ($this->isDisputeCancelled($order)) { //returns true if dispute was cancelled
$order->setStatus('cancelled');
$this->insertRecordInOrderHistory($order,'cancelled');
$order->rollBackRefund(); //cancels the refund on ebay and internally in my system
} else if ($this->isDisputeOlderThan7Days($order)) { //returns true if 7 days elapsed since the dispute was opened
$order->closeDispute(); //closes the dispute on ebay
$this->insertRecordInOrderHistoryTable($order,'refunded');
$order->refund(); //refunds the order on ebay and internally in my system
}
Could be reduced to
var $strategy = $this.getOrderStrategy($order);
$strategy->preProcess();
$strategy->updateOrderHistory($this);
$strategy->postProcess();
where getOrderStrategy wraps the order in a DisputeAcceptedStrategy, DisputeCancelledStrategy, DisputeOlderThan7DaysStrategy, etc. each of which know how to handle the given situation.
Edit, to answer question in comments.
could you please elaborate more on your code. What I understood is that getOrderStrategy is a factory method that returns a strategy object depending on the order status, but what are the preProcess() and preProcess() functions. Also why did you pass $this to updateOrderHistory($this)?
You're focussing on the example, which could be completely inappropriate for your case. I don't have enough detail to be sure of the best implementation, so I came up with a vague example.
The one common piece of code you have is insertRecordInOrderHistoryTable, so I chose to use that (with a slightly more generic name) as the central point of the strategy. I pass $this to it, because it is calling a method on this, with $order and a different string per strategy.
So, basically, I envision each of those looking like this:
public function updateOrderHistory($auth) {
$auth.insertRecordInOrderHistoryTable($order, 'cancelled');
}
Where $order is a private member of the Strategy (remember I said it should wrap the order) and the second argument is different in each class. Again, this might be entirely inappropriate. You might want to move insertRecordInOrderHistoryTable to a base Strategy class and not pass the Authorization class in. Or you might want to do something entirely different, it was just an example.
Likewise, I limited the rest of the differing code to pre- and postProcess methods. This is almost certainly not the best you can do with it. Give it more appropriate names. Split it down into multiple methods. Whatever makes the calling code more readable.
You might prefer to do this:
var $strategy = $this.getOrderStrategy($order);
$strategy->setStatus();
$strategy->closeDisputeIfNecessary();
$strategy->refundIfNecessary();
$strategy->insertRecordInOrderHistoryTable($this);
$strategy->rollBackRefundIfNecessary();
And have some of your Strategies implement empty methods for the "IfNecessary" methods.
Whatever makes the calling code more readable.
You wrote
Sprites contain pixel data
which is a clear sign of a "has a" relationship, not an "is a". So bite the bullet and make Sprite -> Resource a composition.
Why not composition? Because I'd still have to implement methods in Sprite (and similar classes) that essentially call the methods of Resource
yes, you will - but only trivial delegation methods. You won't have to repeat any functional code. That's actually acceptable. and the standard way composition/delegation works. Here you find a detailed example how to replace inheritance by delegation. This approach is so common, there is even an Eclipse refactoring exactly for this purpose,
Best Answer
I don't know that either option is going to be particularly robust. In both cases, you're relying on some kind of singleton or static-state-heavy implementation to manage what amount to global variables. How exactly you access the global variables (whether directly or hidden behind some kind of weakly typed "getData()" call) isn't going to matter a lot to maintainability in the face of a global variable scheme in general. Your application is going to be highly cross-coupled and interdependent either way.
I would say a decent option with an eye on future flexibility would be to take your getData() concept and put it in an interface. Then inject an instance of that interface into classes that need access to the data (and try to limit that number as much as possible). This way you can mock out or swap the thing that provides that data. The way you have it now, this is impossible -- every client of that singleton depends on the entire application state being spun up in order for it to be used, making testing and decoupling difficult, if not impossible. And that state of affairs is the opposite of robust.