ronag
ronag

Reputation: 51263

Decorator or abstract base class? Neither seem right

I'm writing an interface which basically has two methods next which tries to get the next value and prev which returns the previous value.

struct foo
{
    virtual boost::optional<int> next() = 0;
    virtual int prev() = 0;
};

The interface will basically be used by first calling next, and if that fails it is possible to get the previous value with prev. prev is not as trivial as in this example, some other stuff should happen for prev values. The prev method works pretty much the same for all classes and I would like to provide a default implemention of it.

I have to ways of implementing this, neither of which I like:

First possibility is an abstract base class instead of interface.

struct abstract_foo : public foo
{
    int prev_;

    virtual boost::optional<int> next()
    {
        boost::optional<int> val = do_next();
        if(val)
            prev_ = *val;
        return val;        
    }

    int prev()
    {
        return prev_;
    }

    virtual boost::optional<int> do_next() = 0;
}; 

However, it breaks the interface, now one either have to remember to call abstract_foo::next or be forced to implement a rather ugly do_next.

Another way is using a decorator:

struct foo
{
    virtual boost::optional<int> next() = 0;
    virtual int prev()
    {
        throw not_implemented();
    }
};

struct foo_decorator : public foo
{
    std::unique_ptr<foo> foo_;
    int prev_;

    foo_decorator(std::unique_ptr<foo>&& foo)
        : foo_(std::move(foo))
    {
    }

    boost::optional<int> next()
    {
        boost::optional<int> val = foo_->next_value();
        if(val)
            prev_ = *val;
        return val;
    }

    int prev()
    {
        return prev_;
    }
};

Here the, not_implemented just screams for accidents where users think they can use prev directly from the base class.

Does anyone have a good suggestion for good design? Or should I simply have some kind of helper class that help implementing it manually in derived classes?

Upvotes: 1

Views: 173

Answers (2)

Irit Katriel
Irit Katriel

Reputation: 3564

In the second option, why not implement prev already in the base class:

struct foo
{
public:
    virtual boost::optional<int> next() = 0;
    int prev()
    {
        return prev_;
    }
protected:
    int prev_;
};

struct foo_decorator : public foo
{
    std::unique_ptr<foo> foo_;

    foo_decorator(std::unique_ptr<foo>&& foo)
        : foo_(std::move(foo))
    {
    }

    boost::optional<int> next()
    {
        boost::optional<int> val = foo_->next_value();
        if(val)
            prev_ = *val;
        return val;
    }
};

Upvotes: 0

n. m. could be an AI
n. m. could be an AI

Reputation: 120001

Your first option is actually close to the preferred way, which would be as follows:

class foo
{
  public:
    boost::optional<int> next()
    {
        boost::optional<int> val = do_next();
        if(val)
            prev_ = val;
        return val;
    }

    int prev()
    {
        return prev_;
    }

  private:
    int prev_;
    virtual boost::optional<int> do_next() = 0;
}; 

Note how next() and prev() are non-virtual, and do_next() is private. Public non-virtual interface calls private virtual implementation.

Upvotes: 3

Related Questions