Jump to content

How deep should objects be accessed?


NotionCommotion

Recommended Posts

In the ClientCollections class, I have the following:

 
$this->clientList[$connStream]->client->callbackCollections->get($rpcId)->executeSourceResponse($data);
$this->clientList[$connStream]->client is the Client application tied to a given connection, and I think it is good as is.
 
callbackCollections is the list of callbacks associated with the instance of a client, and has a get() method which returns a given callback.
 
All callbacks have the executeSourceResponse() method which performs the applicable task.
 
Should I restructure ClientCollections  so that this level need not know all the gory details such as the following?
 
$this->clientList[$connStream]->client->executeSourceResponse($guid, $data);
And then Client::executeSourceResponse($guid, $data) does:
$this->callbackCollections->executeSourceResponse($guid, $data);
And then CallbackCollections::executeSourceResponse($guid, $data) does:
$this->get($guid)->executeSourceResponse($data);

But then I have these three levels of executeSourceResponse() methods which gets confusing.  Sure, I can rename them, but I don't know how that would make things less confusing.

 

What is considered "good practice" for something like this?

 

Related to this, ClientCollections, Client, and CallbackCollections are currently in the same namespace and may throw a JsonRpcException exception which is caught by ClientCollections.  There are a  bunch of different versions of callbacks which are returned by CallbackCollections::get(), so I put them in their own namespace to keep them organized.  I am debating on allowing these callbacks to be able to also throw an \JsonRpcException exception, or instead have them through their own exception and do something like:

 

try {
     $this->get($guid)->executeSourceResponse($data);
}
catch(Callbacks\CallbackException $e) {
    throw new JsonRpcException($e->getMessage(), $e->getCode());
}

I don't know if this is really getting me anything other than potentially more flexibility in the future.

 

Link to comment
Share on other sites

I would go with something like:

<?php

$client = $this->clientList[$connStream]->Client;
$client->executeSourceResponse($guid, $data);

// Where Client::executeSourceResponse would do:

$this->callbackCollections->get($guid)->executeSourceResponse($data);

It cleanly separates concerns. Whatever class has the ClientList is already aware of ClientList and Client. It shouldn't be made aware of further internals of the Client.

 

This makes sure that if in the future you would re-structure Client, no other classes would need to be updated.

Edited by ignace
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.