CODEWITHSUNDEEP
c++oopdesign-patterns
p.magalhaes
p.magalhaes

Reputation: 8374

Is this an example of bad design?

I will start with my design:

class IOutputBlock{
public:
    virtual void write(char *) = 0;
    virtual bool hasMemory() = 0;
    virtual void openToWrite() = 0;
};

class IInputBlock{
public:
    virtual bool hasNext() = 0;
    virtual IField *next() = 0;
    virtual void openToRead() = 0;
};

class MultiplicationNode : public OperationNode
{
public:
    MultiplicationNode(Node *l, Node *r);
    ~MultiplicationNode(void);
    virtual bool hasNext();
    IInputBlock * evaluate();
};


class IOBlock: public IInputBlock, public IOutputBlock{
    virtual void write(char *);     
    virtual bool hasMemory();
    virtual void openToWrite();
    virtual bool hasNext();
    virtual IField *next();
    virtual void openToRead();

};

Inside the evaluate method i need to create an IOuputBlock to write data in the block. I want the MultiplicationNode consumer just see method for iterate over the block (IInputBlock interface). But ​​in the return of evaluate method, I have to perform a typecast.
Is this implementation correct? Or is it an example of bad design?
Can u suggest another design? Or maybe design pattern to help.

IInputBlock * MultiplicationNode::evaluate()
{
    IOutputBlock *outputBlock = new IOBlock();
    //need to write to outputblock
    return (IInputBlock *)outputBlock;
}

I could also do this below, but I don't think it is right, because i was violation "program to an interface", and exposing unnecessary methods inside evaluate method from IInputBlock interface. 

IInputBlock * MultiplicationNode::evaluate()
{
    IOBlock *outputBlock = new IOBlock();
    //need to write to outputblock
    return outputBlock;
}

Upvotes: 2

Views: 154

Answers (1)

Alexei Levenkov
Alexei Levenkov

Reputation: 100545

One option is to separate read and write classes (even if underlying data is shared):

class WriteOnlyBlock: public IOutputBlock{
    // return new instance of something like ReadOnlyBlock 
    // potentially tied to same internal data
    public: IInputBlock AsRead()...
}

This way you make conversion explicit and prevent callers from attempting to cast IInputBlock to IOutputBlock and minimize number of extra methods exposed by each class.

Upvotes: 2

Related Questions