NotionCommotion Posted February 5, 2018 Share Posted February 5, 2018 I have some various callback objects which are created and placed on a stack. Later, various events happen which initiates the callback object to be retrieved and to perform various methods When complete, I am trying to determine whether the callback must remain in the stack, or can be deleted. My difficulty is only the callback object knows whether its job is done and it can be removed. Originally, I was planning on having the CallbackCollections::get() method have some logic in it to determine whether it should be deleted, but it is starting to get a little too convoluted as the execution of methods in the main space might result in the callback no longer being required. As an alternative solution, I am now thinking of retrieving the callback object and working with it, and then always asking it if it is needs to exist and if not deleting it. Even thought not excessive, I would rather not clutter my script with this check, and have this functionality reside solely in the callback object. I don't know if this is possible, however, as I don't think it is possible for a child object to directly modify its parent's scope. Unless, thinking of it for the first time right now as I type.... I pass the CallbackCollections::stack to the callback's constructor. I don't think it will be a memory hog if I pass the array by reference. A drawback is it shouldn't have the ability to affect other callbacks, but I guess I just make sure I don't include script which allows it to do so Or, maybe.... A better idea is to pass it an anonymous function which will only allow it to remove itself from the stack? Guess this still means ultimately passing it the stack, but seems a little more encapsulated. Probably should be passed on the closure by reference, right? Hope this makes sense. If so, any recommendations? Thank you $callbackCollections=new CallbackCollections(); //Event 1 $callback=$callbackCollections->get($callbackId); // Invoke one or more specific methods on the callback object // Only the callback object knows if it still needs to exist in the stack if($callback->areYouFinished()) { $callbackCollections->delete($callbackId); } //Event 2 $callback=$callbackCollections->get($callbackId); // Invoke other specific methods on the callback object // Only the callback object knows if it still needs to exist in the stack if($callback->areYouFinished()) { $callbackCollections->delete($callbackId); } //Event 3... class CallbackCollections { private $stack=[], $count=0, $error=null; //Last error public function create($method, $httpConn, $httpCallbackId) { if(isset($this->methodMap[$method])) { $this->error=null; return new $this->methodMap[$method]($this->guid, $this->conn, $httpConn, $httpCallbackId, $this->validate, $this->pdo); } else { $this->error="Invalid method $method"; return false; } } public function delete($callbackId) { unset($this->stack[$callbackId]); } public function getError() { return $this->error; } public function add($callback) { $callbackId=++$this->count; $callback->setCallbackId($callbackId); $this->stack[$callbackId]=$callback; return $callbackId; } public function get($callbackId, bool $delete=false) { if(isset($this->stack[$callbackId])) { $callback=$this->stack[$callbackId]; if($delete || $callback->autoRemove()) unset($this->stack[$callbackId]); } else { $callback=false; $this->error="Callback does not exist"; } return $callback; } } Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted February 6, 2018 Author Share Posted February 6, 2018 Ended up going like this... Haven't tested it yet, but should be fine. public function add($callback) { $callbackId=++$this->count; $callback->setCallbackId($callbackId,function()use($callbackId){unset($this->stack[$callbackId]);}); $this->stack[$callbackId]=$callback; return $callbackId; } Quote Link to comment Share on other sites More sharing options...
requinix Posted February 6, 2018 Share Posted February 6, 2018 Do the callbacks have return values yet? If not then they can return true/false for whether they should remain active. Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted February 6, 2018 Author Share Posted February 6, 2018 Do the callbacks have return values yet? If not then they can return true/false for whether they should remain active. Some do, some do not. Originally, I was using an anonymous function for all my callbacks, but changed to a simple class and was very happy I did. Any issues with my proposed solution. Has been tested and seems to work perfectly. My only concern is memory usage since it is being implemented in a server application. Quote Link to comment Share on other sites More sharing options...
requinix Posted February 6, 2018 Share Posted February 6, 2018 Seems alright to me. I'm not decided on exactly how I would manage callbacks with the callback collection, but I think I would take a similar approach that you have: tell the callback its ID and have it unregister itself from the collection. I do think I would rather have a public removal method than to accomplish that with yet another closure. Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted February 6, 2018 Author Share Posted February 6, 2018 (edited) I do think I would rather have a public removal method than to accomplish that with yet another closure. I agree one shouldn't use closure just for the sake of using closure. I remember a while back angry guy getting so upset that I was doing so. I recently came to an epiphany, and recognize he was correct and making an object based on an instance a class as I ended up doing provided more flexibility and simplicity. But for this case, without using closure, how do I best accomplish it? In which class would that "public removal method" be located? Couldn't be in CallbackCollections as it wouldn't be able to be accessed by any given callback. Are you recommending passing CallbackCollections::stack to the callbacks in its constructor? EDIT. Then, why did you specifically say "public" removal method as it could just as well be protected? Note. Upon review, I will likely not first create the callback, then load it with data, and then add it to the stack if the data is valid, but do so in one step. $callbackCollections=new CallbackCollections(); if(!$callback=$callbackCollections->create($method, $httpConnStream, $httpRpcId)) { //error } elseif($errors=$callback->load($params, $extra)) { //error } else { $rpcId=$callbackCollections->add($callback); $callback->doSomething(); } // .... if(!$callback=$callbackCollections->get($id)) { //error } else { $response=$callback->doSomethingElse($result); } class MaintenanceFileEdit extends Callback { protected $method='maintenance.FileEdit'; public function doSomethingElse($result) { ($this->removeFromStack)(); } } abstract class Callback implements CallbackInterface { protected $guid, $sourceConn, //Of source client. $removeFromStack, //Anonomous function to delete from stack $sourceRpcId, //Of source client (i.e RPi). Not set until after load $serverConn, //Of calling machine (i.e. the HTTP server) $serverRpcId, //Of calling machine (i.e. the HTTP server) $pdo, $validate, $params=[], //Only required for deferred callbacks $extra=[], //Only required for deferred callbacks $timestamp; public function __construct($guid, $sourceConn, $serverConn,$serverRpcId,$validate=false,$pdo=false) { $this->guid=$guid; $this->sourceConn=$sourceConn; $this->serverConn=$serverConn; $this->serverRpcId=$serverRpcId; //$sourceRpcId must be set after it is added to stack $this->pdo=$pdo; $this->validate=$validate; $this->timestamp=time(); //Only maybe needed for reports } public function load($params,$extra=[]) { //Returns error array upon error if($errors=$this->validateForwardRequest($params)) { $this->params=null; $this->extra=null; return $errors; } else { $this->params=$params; $this->extra=$extra; //Extra not validated } } public function setRpcId($sourceRpcId, $deleteAnonomousFunc) { $this->sourceRpcId=$sourceRpcId; $this->removeFromStack=$deleteAnonomousFunc; //Used to later delete callback from stack } public function doSomethingElse($result) { //... } //Other classes as needed } class CallbackCollections { private $stack=[], $count=0; public function create($method, $httpConn, $httpRpcId) { if(isset($this->methodMap[$method])) { $this->error=null; $class='DataLogger\Server\Callbacks\\'.$this->methodMap[$method]; return new $class($this->guid, $this->conn, $httpConn, $httpRpcId, $this->validate, $this->pdo); } else { $this->error="Invalid method $method"; return false; } } public function delete($rpcId) { unset($this->stack[$rpcId]); } public function add($callback) { $rpcId=++$this->count; $callback->setRpcId($rpcId,function()use($rpcId){unset($this->stack[$rpcId]);}); //Used to allow object to remove itself from the stack when complete. $this->stack[$rpcId]=$callback; $this->debugStack('add'); return $rpcId; } public function get($rpcId) { if(isset($this->stack[$rpcId])) { $callback=$this->stack[$rpcId]; } else { $callback=false; $this->error="Callback does not exist"; } $this->debugStack('get'); return $callback; } } Edited February 6, 2018 by NotionCommotion Quote Link to comment Share on other sites More sharing options...
requinix Posted February 7, 2018 Share Posted February 7, 2018 But for this case, without using closure, how do I best accomplish it? In which class would that "public removal method" be located? Couldn't be in CallbackCollections as it wouldn't be able to be accessed by any given callback. Are you recommending passing CallbackCollections::stack to the callbacks in its constructor?I'm not necessarily recommending it, I'm just saying I think that's how I would do it. It's starting to treat them less like callback functions and more like functors - objects with a singular callable purpose, but still objects. Then, why did you specifically say "public" removal method as it could just as well be protected?It couldn't be protected. That only allows the class and descendant classes to use it, and the callback class would not be a descendant of the callback collection class. So it would have to be public. At this point it's hard for me to recommend anything because we're straddling the line between abstract concepts (how I've been thinking) and a concrete implementation (what you need). I can throw out ideas but it's up to you to figure out the most suitable approach. Quote Link to comment Share on other sites More sharing options...
kicken Posted February 7, 2018 Share Posted February 7, 2018 I can think of a few various ways that you might accomplish your end goal, but which way would be ideal depends a lot on how your system is currently constructed, which is unclear to me. If your callbacks are currently being registered by various objects in some way then you could just have that registration process return an object that permits unregistration of the callback (and maybe other actions). This seems to be similar to what you were doing in your last post. class CallbackCollection { private $stack = []; public function register($callback){ $this->stack[] = $callback; return new CallbackWrapper($this, $callback); } public function unregister($callback){ $this->stack = array_filter($this->stack, function($f) use ($callback){ return $f !== $callback; }); } public function trigger(...$args){ foreach ($this->stack[] as $f){ call_user_func($f, ...$args); } } } class CallbackWrapper { private $collection; private $callback; public function unregister(){ $this->collection->unregister($this->callback); } } class MyCallback { private $registration; public function __construct($collection){ $this->registration = $collection->register([$this, 'onSomeEvent']); } public function onSomeEvent(){ if ($this->shouldUnregister()){ $this->registration->unregister(); } } } If your callbacks are just simple closure functions you could pass it an argument that permits it to unregister itself. This could be done either by setting a flag and having the collection do the removal, or passing an unregistration closure that the callback invokes. Setting a flag might go something like this: class SomeEvent { public $data; public $remove; public function __construct($data){ $this->data = $data; } } class CallbackCollection { //... public function trigger($event){ foreach ($this->stack as &$f){ call_user_func($f, $event); if ($event->remove){ $f = null; } } $this->stack = array_filter($this->stack); } } $collection = new CallbackCollection(); $collection->register(SomeEvent::NAME, function(SomeEvent $event){ if ($done){ $event->remove = true; } }); Or as a closure class CallbackCollection { //... public function trigger(...$args){ foreach ($this->stack as &$f){ call_user_func($f, ...$args, function() use (&$f){ $f = null; }); } $this->stack = array_filter($this->stack); } } $collection = new CallbackCollection(); $collection->register(function($data, $remove){ if ($done){ $remove(); } }); Maybe that'll help spark some ideas for you. Figure out what works best with your current setup and go with that. Quote Link to comment Share on other sites More sharing options...
kicken Posted February 7, 2018 Share Posted February 7, 2018 As another option to consider, do you actually need to remove your callbacks from the stack, or could you just have them do nothing when called after whatever condition is fulfilled? For example: $collection->register(function(){ static $done; if ($done){ return; } //... if ($whatever){ $done = true; } }); Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted February 7, 2018 Author Share Posted February 7, 2018 Thanks requinix and kicken, As a result of this dialog, I actually have a concrete implementation. Actually several. They are all to some degree the same and require that the parent (i.e. CallbackCollection) pass something about itself to the individual callback's constructor. This "something" can be $this, an anonymous function, a wrapper, the stack array, etc. If you disagree with this conclusion, please advise, otherwise I think I am good. Thank you requinix for the link to functors. As my callbacks have multiple functions (and thus methods), I don't know yet if they are applicable, but it is a term (and maybe a concept) which I wasn't aware of, and will likely be of use later. Thank you kicken about showing how the wrapper could be used. As a matter of principle, I didn't wish to pass the callback the whole tamale (i.e. CallbackCollection's $this), and the wrapper allows me to limit scope. I don't think I would want leave them indefinitely on the stack for memory conservation reasons. 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.