Implications of changing an object from the inside?

[NotionCommotion]

Upon connection, I am adding a client who's single purpose in life is to authenticate registration and if valid change the client to the appropriate type.  Originally, I planned on giving it access to the parent client collection's $this->clientList, object, however, I just realized I am just giving it access to itself (i.e. $this) by using $this->clientList[$this->connStream]->Client.  Will I be running into trouble?

 

Off topic, but what would be the appropriate way to control access other than my approach with Client::executeRequest($method, $params) and UnregisteredClient::getMethod($method)?

 

Thanks

 

$server->on('connection', function (\React\Socket\ConnectionInterface $conn) {
    $connStream = new LengthPrefixStream($conn);
    $this->clientController->addClient($connStream);
    $connStream->on('data', function($data) use ($connStream){
        $this->clientController->processMessage($connStream,$data);
    });
});

class ClientCollections extends Helpers implements ClientCollectionsInterface
{
    public function __construct() {
        $this->clientList = new \SplObjectStorage();
    }


    public function addClient($connStream) {
        $this->clientList->attach($connStream, new \stdClass);
        $this->clientList[$connStream]->Client=new UnregisteredClient($connStream, $this->clientList);
    }


    public function closeConnection($connStream) {
        if($this->clientList->contains($connStream)) {
            $this->clientList->detach($connStream);
        }
    }


    public function processMessage($connStream, $data) {
        //extract $method and $params
        $result=$this->clientList[$connStream]->Client->executeRequest($method, $params);
    }


}

class UnregisteredClient extends Client
{


    public $type='UnregisteredClient';


    //Only function is to replace client with the correct one
    protected $connStream, $clientList;


    public function __construct(LengthPrefixStream $connStream, \SplObjectStorage $clientList) {
        $this->connStream=$connStream;
        $this->clientList=$clientList;
    }


    protected function getMethod($method) {
        $validMethods=['register'=>'register'];
        return isset($validMethods[$method])?$validMethods[$method]:false;
    }


    ####### Specific Client Type Methods  ############


    protected function register($params) {
        //Identify...
        $this->clientList[$this->connStream]->Client=new SomeOtherClient();
    }
}

abstract class Client
{
    public function executeRequest($method, $params) {
        if(!$validMethod=$this->getMethod($method)) throw new \Exception("Invalid method '$method'.", -32601);
        return $this->$validMethod($params);
    }
}

 

[NotionCommotion]

I think I will likely need to change to something like the following.  I guess I could even use ClientCollections as the controller as well.  Still need a way to limit which methods can be accessed.  While my getMethod() approach will work, it just doesn't seem like the most professional way to do so.

 

class ClientCollections extends Helpers implements ClientCollectionsInterface
{
    public function __construct($registrationController) {
        $this->clientList = new \SplObjectStorage();
        $this->registrationController = $registrationController;
    }

    public function addClient($connStream) {
        $this->clientList->attach($connStream, new stdClass);
    }

    public function getController($connStream) {
        return existingController()?$this->clientList[$connStream]:$this->registrationController;
    }

    public function processMessage($connStream, $data) {
        //extract $method and $params
        $controller=$this->getController($connStream);
        $controller->executeRequest($method, $params);
    }

}

[ignace]

This design has no cohesion. If you look at ClientCollection and UnregisteredClient you see they are both aware of the internals of your clientList, that it has a property Client and that it is a stdClass.

 

Every class involved is made aware of a LengthPrefixStream.

 

Do not branch out of your code until it is absolutely necessary. Keep the clientList and connStream and all of that inside $server.

 

With your Client class you are on the right track.

[NotionCommotion]

Thanks ignace, After wondering down my described path for a while, I came to the same conclusion.