Skip to content

Instantly share code, notes, and snippets.

@Danack
Last active November 9, 2015 15:04
Show Gist options
  • Save Danack/989d23600ccab991be0d to your computer and use it in GitHub Desktop.
Save Danack/989d23600ccab991be0d to your computer and use it in GitHub Desktop.
Multiple dispatch is required.
<?php
// OPs code is trying to put both of these things into a single interface:
interface uploader {
public function createProduct($product, $multi);
or
public function createProduct(array $products, $multi, $variations);
}
// The problem is that these are just inherently incompatible interfaces. Although you could
// theoretically come up with an abstracted interface, that would almost certainly be horrible
// as you the abstraction would just be incorrect and misleadingly so for at least once of those objects.
// What you need to recognise is that this is a case where the application is building up information
// during it's operation. In particular, the functions that are required to do the uploading cannot
// be determined until some code is run. The OP is trying to solve this problem by making the two different
// sets of function have the same interface, so that it doesn't matter which is chosen to be run during the
// operation of the application, either set of functionality can be swapped in for another.
// The much better solution is just to acknowledge that this is an application that needs to build up
// information of what needs to be executed _INTERNALLY_ to the programs execution, and to work with a
// framework that has that. *cough* https://github.com/danack/tier Below is how I would refactor that code
// to do that.
// 1st stage refactor - acknowledge that the interface isn't common and should be done by two separate
// functions. Lets imagine that
// the use-case is that someone has a webpage where they can upload
// some images and text, with prices and then select to upload that to either amazon or ebay or both.
// So the first part of the program is to figure out which uploaders need calling, and the uploading part
// is separate
function uploadAmazonProducts(AmazonClient $ac, ProductList $productList) {...}
function uploadEbayProducts(EbayClient $ec, ProductList $productList) {...}
function determineUploaders(UserInput $userInput) {
$uploaderList = [];
if ($userInput->isAmazonSelected() == true) {
$uploaderList[] = 'uploadAmazonProducts'
}
if ($userInput->isEbaySelected() == true) {
$uploaderList[] = 'uploadEbayProducts'
}
return $uploaderList;
}
$injector->delegate(ProductList::class, 'createProductListFromUserInput')
$uploaderList = $injector->execute('determineUploaders');
foreach ($uploaderList as $uploader) {
// We execute each of the uploaders as appropriate. The first stage of 'determineUploaders' has no
// knowledge of what the 'uploader' callables require, only their name. The injector does all the
// work to provide the dependencies for each uploader.
$injector->execute($uploader);
}
//2nd stage refactor - so making the functions that uploaded to Amazon/Ebay be separate is nice...but
//I would strongly suspect that the 'ProductList' is also a bad abstraction. Stuff that is possible to do
// on Amazon is not possible to do on Ebay. So again, using a common abstraction between the two leads to
// at least one of the abstraction being either misleading or flat out wrong.
//
// So lets separate the two of them as well, using the delegate functionality.
// These factory functions are not abstract at all, they create ProductLists specific to each retailer.
function createAmazonProductListFromUserInput(UserInput $ui) {
return AmazonProductList::fromUserInput($ui);
}
function createEbayroductListFromUserInput(UserInput $ui) {
return EbayProductList::fromUserInput($ui);
}
//Now set the uploading functions to have dependencies on their specific ProductList
function uploadAmazonProducts(
AmazonClient $ac,
AmazonProductList $productList
) {...}
function uploadEbayProducts(
EbayClient $ec,
EbayProductList $productList
) {...}
//Tell the injector how to create each of those specific product lists.
$injector->delegate('AmazonProductList', 'createAmazonProductListFromUserInput');
$injector->delegate('EbayProductList', 'createEbayProductListFromUserInput');
//This code is the same as before.
function determineUploaders(UserInput $userInput) {
$uploaderList = [];
if ($userInput->isAmazonSelected() == true) {
$uploaderList[] = 'uploadAmazonProducts'
}
if ($userInput->isEbaySelected() == true) {
$uploaderList[] = 'uploadEbayProducts'
}
return $uploaderList;
}
$uploaderList = $injector->execute('determineUploaders');
foreach ($uploaderList as $uploader) {
$injector->execute($uploader);
}
// Yay! We have perfectly understandable code, without any need for abstractions!
// Don't get me wrong - abstractions are lovely when they are correct. But when they are inherently wrong
// you shouldn't force yourself to use them, just because your current framework sucks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment