NotionCommotion Posted January 16, 2019 Share Posted January 16, 2019 The client provides name/value pairs which are used to update something and I am currently using switch statements to direct the action based on the provided name, however, it is becoming increasingly more complicated. Are there any standard approaches to doing so? I am thinking of doing the following. Any concerns? Thanks $c['fooService'] = function ($c) { return new Foo\FooService( new Foo\FooMapper(), new ApiConnector(), new Foo\FooNameValueUpdater(), ); }; $app->put('/foo/{id:[0-9]+}', function (Request $request, Response $response, $args) { return $this->fooResponder->update($response, $this->fooService->update($args['id'], $request->getParsedBody())); }); Class FooService extends BarService { public function __construct($mapper, $updater, $apiConnector) { $this->mapper = $mapper; $this->apiConnector = $apiConnector; $this->updater = $updater; } public function update($id, $params) { //$params=['name'=>'property1name', 'value'=>123] $this->updater->$params['name']($id, $params['value'], $this); } public function updateSomeStandardAction($id, $value, $propertyName) { //some code... $this->mapper->bla($id, $value, $propertyName); } public function updateSomeOtherStandardAction($id, $value, $propertyName) { //some code... $this->apiConnector->bla($id, $value, $propertyName); } } Class FooNameValueUpdater extends BarNameValueUpdater { public function property1name($id, $value, $service) { $this->updateSomeStandardAction($id, $value, $service, 'property1name'); } public function property2name($id, $value, $service) { $this->updateSomeStandardAction($id, $value, $service, 'property2name'); } public function property3name($id, $value, $service) { $this->updateSomeStandardAction($id, $value, $service, 'property3name'); } public function property4name($id, $value, $service) { $this->updateSomeStandardAction($id, $value, $service, 'property4name'); } protected function updateSomeStandardAction($id, $value, $service, $propertyName) { //some code... $service->updateSomeStandardAction($id, $value, $propertyName); } protected function updateSomeOtherStandardAction($id, $value, $service, $propertyName) { //some code... $service->updateSomeOtherStandardAction($id, $value, $propertyName); } } Quote Link to comment Share on other sites More sharing options...
requinix Posted January 16, 2019 Share Posted January 16, 2019 If you still need the ability to customize various actions and it's just that those property1-4name ones all work the same way, public function __call($method, $args) { if (in_array($method, ['property1name', 'property2name', 'property3name', 'property4name'])) { $this->updateSomeStandardAction($args[0], $args[1], $args[2], $args[3], $method); } else { throw new \BadMethodCallException("No such method or property '{$method}'"); } } Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted January 16, 2019 Author Share Posted January 16, 2019 One originally thinks they all work the same way and then there is some new requirement and then another, and before you know it... I intended to show two of them as "updateSomeOtherStandardAction" but failed to do so. Did my proposed solution make any sense? Quote Link to comment Share on other sites More sharing options...
requinix Posted January 16, 2019 Share Posted January 16, 2019 I don't know how it differs from your current code... Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted January 16, 2019 Author Share Posted January 16, 2019 I didn't post my existing code but just said it used a switch statement. The following method is located in the controller/service and does the name to method routing. I want to allow inheritance which makes it even uglier. Some of the endpoints need to initiate multiple processes so I sometimes return a single method and other times return an array of methods. My new approach is using a dedicated class for the routing which will throw an error if the method doesn't exist. The part I've never done on my new approach is have the service pass itself to the methods in the Updater method. Originally I was planning on just injecting the mapper when creating the Updater but the mapper doesn't contain all the needed functionality. protected function getUpdateMethods(string $prop) { switch ($prop) { case 'name': case 'virtualLansId': return 'updateValueDatabase'; case 'guid': return ['updateValueDatabase','disconnect']; case 'reconnectTimeout': case 'responseTimeout': case 'historyPackSize': return ['updateValueDatabase','updateClient']; default: return parent::getUpdateMethods($prop); } } Quote Link to comment Share on other sites More sharing options...
requinix Posted January 16, 2019 Share Posted January 16, 2019 It feels a bit painful but I would keep the switch. However I wouldn't use a dynamic dispatch (method names as strings then ->$method) but actually perform the update method calls in the switch. No "getUpdateMethods" but instead "update($prop, $value, $service)". switch ($prop) { case 'name': case 'virtualLansId': $this->updateValueDatabase($prop, $value, $service); break; case 'guid': $this->updateValueDatabase($prop, $value, $service); $this->disconnect(...); break; case 'reconnectTimeout': case 'responseTimeout': case 'historyPackSize': $this->updateValueDatabase($prop, $value, $service); $this->updateClient(...); break; default: parent::update($prop, $value, $service); break; } Of course, if all of them go through updateValueDatabase() then you can pull that out of the switch. At some point you'll have to map properties to methods. There are very many "magical" ways you can go about it, but they're all complicated, and some complicated to the point that they're too obscure for anyone but the author to work with. At some point it's better to give up on the magic and embrace reality: some simple code that's easy to find and understand is capable of doing the job well enough. Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted January 16, 2019 Author Share Posted January 16, 2019 22 minutes ago, requinix said: At some point you'll have to map properties to methods. There are very many "magical" ways you can go about it, but they're all complicated, and some complicated to the point that they're too obscure for anyone but the author to work with. At some point it's better to give up on the magic and embrace reality: some simple code that's easy to find and understand is capable of doing the job well enough. I agree my dynamically dispatched methods had a lot to be desired in their original form. My new approach is definitely better as I don't need to use parent::update($prop, $value, $service); and have an object with methods. That being said, I may go with your recommended updateValueDatabase ($prop, $value, $service);. Also, not only are some of these approaches too complicated and obscure for anyone but the author, in my case they are too complicated and obscure for myself Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.