Jump to content

Should the request hold the response, the response hold the request, or both (or neither)??


NotionCommotion

Recommended Posts

I have an application which receives multiple requests at once and it advantageous to process them as a group.

I am thinking I should be doing something like the following, however, as I just came up with it on my own, would appreciate a critique.  Is this a common design pattern or should I be doing something differently?  Note that I have three questions asking specific questions in the below code (look for the comments with question marks).

Thanks
 

<?php
class Request
{
    private $response, $data;

    public function __construct(array $data)
    {
        $this->data=$data;
        return $this->response=new Response($this);
    }    

    public function getResponse():Response
    {
        return $this->response;
    }

    public function updateResponse($results):self
    {
        $this->response->setResults($results);
        return $this;
    }
}
<?php
class Response
{
    private $request, $results;

    public function __construct(Request $request)
    {
        //I don't really see the need for this.  Any need?
        $this->request=$request;
    }

    public function setResults($results):self
    {
        $this->results=$results;
        return $this;
    }

    public function getResults()
    {
        return $this->results;        
    }
}

 

<?php
class Container implements \Iterator
{
    private $position = 0;
    private $requests, $response;

    public function __construct(WorkerObject $worker, array $inputArray)
    {
        $this->requests=[];
        $this->responses=[];
        foreach($inputArray as $i =>$inputData) {
            $this->requests[$i]=new Request($inputData);
            $this->responses[]=$this->responses[$i]->getResponse();
        }

        //Should WorkerObject be responsible to execute Request::updateResponse() method? 
        $worker->process_requests(...$this->requests);

        //Or should I iterate over my requests upon completion and execute updateResponse() method? 
    }

    public function __construct() {
        $this->position = 0;
    }

    public function rewind() {
        $this->position = 0;
    }

    public function current() {
        return $this->responses[$this->position];
    }

    public function key() {
        //Will this work?
        return $this->requests[$this->position];
    }

    public function next() {
        ++$this->position;
    }

    public function valid() {
        return isset($this->responses[$this->position]);
    }
}
<?php
//...
$container=new Container(new WorkerObject, $inputArray);
foreach($container as $request => $response) {
    //...
}

 

Link to comment
Share on other sites

1 hour ago, requinix said:

Requests containing responses is common.
Responses containing requests is probably not that common.
Neither containing the other is also an option.

I'd go for either the first or the last of those.

The only potentially reason I could see of containing the request in the response is allows the response to see the request but I surely would want to put a wrapper around the request to prevent modification and will not bother with it.

1 hour ago, benanamen said:

A quick observation....

You have tightly coupled the Request and Response Classes. It would be better to use Dependency Injection (DI) to give the class what it needs by passing the Response instance to the Request Class.

Agree it is tightly coupled, but why do you feel it shouldn't be?  Maybe I am wrong, but my thought is there can never be a response unless there is a request and....  Never mind, I think you are correct.

Link to comment
Share on other sites

21 minutes ago, NotionCommotion said:

The only potentially reason I could see of containing the request in the response is allows the response to see the request but I surely would want to put a wrapper around the request to prevent modification and will not bother with it.

But why would the response need to know about the request? Any logic in creating the response should be handled by a controller, not by the response itself.

And yes, by that logic, the request doesn't even need to know the response either.

Link to comment
Share on other sites

13 hours ago, requinix said:

But why would the response need to know about the request? Any logic in creating the response should be handled by a controller, not by the response itself.

And yes, by that logic, the request doesn't even need to know the response either.

Sure, the controller, however, I need to simplify the association between the request and response.  I have two parts of the application which need to know the association and there are many requests which are processed in multiple groups and the process is complicated.  I could use SplObjectStorage, however, doing so would require many updates which isn't perfect.  My thought was create a response in one part of the application and save a reference to it, include that response in the request, process the requests and when each is complete, update each request's individual response.  Another thought is to basically do the same but use class RequestResponsePair which holds the two.

Link to comment
Share on other sites

On 12/21/2019 at 12:52 PM, requinix said:

You're passing the request to some method, right?.

Yes only if you are including the constructor method in the "some method" group.  The reason I think I should be doing so is my original statement "I have an application which receives multiple requests at once and it advantageous to process them as a group."

Link to comment
Share on other sites

If it needs to have access to all the requests and responses at once then you kinda have to go the RequestResponsePair route (though I'd pick a different name).

But first I would try to see if maybe it could be refactored so that it can do a sort of "set up" step, process each request/response in turn, then "tear down" after. Depends why and how it's processing as a group.

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.