Jump to content

Updating using name/value pairs


NotionCommotion

Recommended Posts

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);
    }
}

 

Link to comment
Share on other sites

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}'");
	}
}

 

Link to comment
Share on other sites

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);
        }
    }


 

Link to comment
Share on other sites

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.

Link to comment
Share on other sites

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 :)

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.