XzKto
XzKto

Reputation: 2502

Proper way to implement "layered" class logic

I must admit that I don't know if I have a problem in my design pattern, over-thinking or maybe even just class naming, so any suggestions are welcome. This must be an easy problem but it is rather hard for me to even explain it in words(language barrier), so I will try to do it in code.

For example I'm trying to iterate over a network data stream byte by byte(the task is completely irrelevant to question, just to better explain pseudo-code, please don't say that I use wrong tool to do that). Base class will look something like this:

class BaseNetworkIterator implements Iterator {

    protected $stream;
    protected $currentByte;

    public function __construct() {
        $this->stream = open_network_stream();
    }

    /** .... * */
    public function next() {
        $this->currentByte = $this->stream->getByte();
    }

    public function current() {
        return $this->currentByte;
    }

}

Now imagine that logic in this class is complicated by itself(not one line per method and there are many more helper methods) but I need to do even more. For example I need to reverse bits order in every byte. I don't want to include this logic in base class as it is already big, so I extend it:

class ReverseByte extends BaseNetworkIterator {
    /** .... * */
    public function next() {
        parent::next();
        $this->currentByte = $this->reverseStreamByte($this->currentByte);
    }

    protected function reverseStreamByte($byte) {
        /** .... * */
    }
}

And more, I want to completely skip all newlines in this stream(reversed):

class SkipNewLine extends ReverseByte {
    /** .... * */
    public function next() {
        do {
            parent::next();
        } while ($this->currentByte === "\n");
    }

}

And if we get two consecutive bytes(after all this operations - they may be separated by newlines etc.) equal to "NO", then we must reverse one following byte once more(don't think about it: just something that uses all previous logic and a protected method that was defined in one of the previous classes):

class HandleDoubleReverseKeyword extends SkipNewLine {

    const DOUBLE_REVERSE_KEYWORD = 'NO';

    /** .... * */
    public function next() {
        parent::next();
        if ($this->getTwoCharsCache() === self::DOUBLE_REVERSE_KEYWORD) {
            $this->currentByte = $this->reverseStreamByte($this->currentByte);
            $this->clearTwoCharsCache();
        } else {
            $this->saveMaxTwoCharsCache($this->currentByte);
        }
    }

    private function saveMaxTwoCharsCache($newChar) {
        /** .... * */
    }

    private function getTwoCharsCache() {
        /** .... * */
    }

    private function clearTwoCharsCache() {
        /** .... * */
    }

}

And now although logic is split between classes rather nicely for my taste(each consecutive class does something completely different and isolated and it is easy to see what is done in each layer) it is a mess to work with. If we add new "filter" in the middle we must change what later class is extended from... Naming starts to get insane as "HandleDoubleReverseKeyword" doesn't explain what this class really does even remotely (even if we are in "NetworkIterator" namespace). I don't really understand what a "final" class should look like.

I was thinking of using traits as naming goes much easier there but they cause more problems then solve(execution order, code completion etc.).

I was also thinking of using Yii-like "behaviour/event" pattern (Something like: base next() method consecutively calls an array of class/methods and some external source can add more class/methods to that array instead of extending base class) but there is a huge problem with properties/methods visibility(it is a problem, basically I have to make ALL properties and methods public) and more importantly it is not clear how to properly access methods of other additional classes.

Ok, this still may be not very clear so any feedback is appreciated. I know there is no question mark in this "question", but I hope my intentions are clear and the only thing that comes to mind is: how to do this type of design properly?

Upvotes: 0

Views: 107

Answers (2)

rae1
rae1

Reputation: 6144

I think you are approaching this from the wrong perspective.

The current hierarchy you have in place highly limits the re-usability of each component, since they are all implemented to work only on the result of a byte stream iterator. If for example you needed to implement a file iterator, and then skip over each new line, you cannot possibly reuse your existing SkipNewLine, because it works only on the result of a reverse byte iterator.

Another fallacy is that you have no way to control the order in which each of this steps occurred without rewriting the entire class structure. Even something as simply as moving the SkipNewLine class before the ReverseByte (or after the HandleDoubleReverseKeyword) is a refactoring nightmare.

I suggest you implement each step in the iteration as a separate class, that solely acts on a generic stream, and makes no assumptions as to the state of the source stream. You can inject an instance of each of these components into your BaseNetworkIterator, and use them independently of each other.

This will further provide you with the ability to act on each of the individual states with knowledge of the global state of the iteration.

Upvotes: 1

Matt
Matt

Reputation: 6050

My 2 cents:

  1. The stream should be separated from the iterator class and its sub classes. Iterator defines how to access the data, each iterator instance should only hold the reference to the data, not the actual data.

  2. The logic in class HandleDoubleReverseKeyword is about how to process the data, not how to access the data; and the action is decided base on what the data is, so the class should not inherited from the iterator class; here I think we can use the state pattern.

So the code to deal with the stream could be like this:

$stream = open_network_stream();
$iterator it = new Iterator(&stream);
$data = it->next(); 
while(data){
  switch(data->value){
     case 'A': AcitionA->action();break;
     case 'B': AcitionB->action();break;
     }
$data = it->next();
}

Upvotes: 1

Related Questions