10

I am working on a slide-based application in C++. Each slide has a slide-items collection which can include items like caption, button, rectangle, etc.

Only some of these items support fill, while others don't.

What is the best way to implement the fill for the slide items in this case? Here are two ways that I thought of:

  1. Create an interface Fillable and implement this interface for slide items which support fill, keeping all the properties related to fill in the interface. When iterating over the list of slide items, dynamic_cast them into Fillable, and if successful, do the operation related to fill.

  2. Make a fill class. Make a fill pointer a part of slide item class, assign the fill object to the fill pointer for those objects which support fill, and for rest of them keep it null. Give a function GetFill, which will return the fill for the items if it exists otherwise returns NULL.

What's the best approach for this? I'm interested in performance and maintainability.

3
  • Do you need to know if the objects are fillable or not, or do you only want to fill if possible? E.g. a GUI might only show the "fill" option when appropriate - and in that case you need both Fill-operation and supports-Fill-predicate; or perhaps this is not relevant. May 7, 2018 at 9:14
  • In this application , the painters are responsible for drawing all the items in the GUI. If they encounter a object with fill , they call specific object fill handlers like gradient or solid fill to draw the insides of the object. Also, based on the fill type the user is shown the appropriate UI to edit the fill when objects are selected. For example :- changing gradient transition points UI in case of objct having gradient fill. May 9, 2018 at 19:46
  • Why not use a free function and let it react to the object that's being "filled"?
    – Clearer
    May 11, 2018 at 11:12

6 Answers 6

10

I would do a combination of the two. Make your Fillable interface and have it be the return type for your GetFill method. This is better than the dynamic cast approach. Using dynamic cast to query for the interface requires that the actual slide item object implement the interface if it is to support it. With an accessor method like GetFill however, you have the option of providing a reference/pointer to some other object that implements the interface. You can also just return this if the interface is in fact implemented by this object. This flexibility can help avoid class bloat and promote the creation of re-usable component objects that can be shared by multiple classes.

Edit: This approach also works nicely with the null object pattern. Instead of returning a null pointer for the objects that don't support Fillable, you can return a simple no-op object that implements the interface. Then you don't have to worry about always checking for null pointers in the client code.

1
  • i agree with this statement, it seems viable May 11, 2018 at 12:03
2

The answer is it depends.

I don't see the point in having to clutter your base interface with fill/get_fillable_instance/... if not every object is supposed to handle fill. You can however get away with just

struct slide_object
{
    virtual void fill() {} // default is to do nothing
};

but it depends on whether you think fill should appear in the slide object abstract class. It rarely should however, unless being non fillable is exceptional.

Dynamic casting can be correct in the case you need to provide two distinct classes of objects (and no more than two), some of them being fillable, and the other having nothing to do with fillability. In this case, it makes sense to have two sub-hierarchies and use dynamic casting where you need.

I have used this approach successfully in some cases and it is simple and maintainable, provided the dispatch logic is not scattered (ie. there is only one or two places where you dynamic cast).

If you are expected to have more fill-like behavior, then dynamic_cast is a wrong choice since it will lead to

if (auto* p = dynamic_cast<fillable*>(x))
   ...
else if (auto* p = dynamic_cast<quxable*>(x))
   ...

which is bad. If you are going to need this, then implement a Visitor pattern.

0

Create a base class SlideItem:

class SlideItem {
    public:
        virtual ~SlideItem();
        virtual void fill() = 0;
};

Then do an empty implementation for those you can't fill:

class Button : public SlideItem {
    public:
        void fill() { }
};

And a proper fill implementation for the others:

class Rectangle : public SlideItem {
    public:
        void fill() { /* ... fill stuff ... */ }
};

and put all of them inside a container.. if you want to fill them just call everybody... easy to maintain.. and who cares about performance :)


If you really need fast code your first solution is certainly good. But if you do it like that, make sure you don't have to cast it every time you want to fill. Cast them one time and put the pointers in a fillable-container. Then iterate over this fillable-container if you have to fill.

Then again, IMHO you put too much effort into this, without a reasonable performance gain. (of course I don't know your application, it might be justified.. but usually not)

3
  • 3
    This is called a fat interface, and it is an antipattern. Sep 7, 2011 at 14:52
  • @Armen Tsirunyan: Ok, right. If there is an antipattern there must be the proper pattern for this also. You can certainly explain it to me :) Oh and if your solution contains twice as many classes (adapters or what ever) I'm not interested..
    – duedl0r
    Sep 7, 2011 at 21:56
  • @duedl0r: I am afraid I read the whole paragraph wrong, sorry. Sep 8, 2011 at 6:06
0

It seems like what you're looking for is close to the Capability Pattern. Your #2 is close to this pattern. Here's what I would do:

Make a fill class. Make fill pointer a part of slide item class, assign the fill object to fill pointer for only those objects which support fill, for rest of them keep it null. Create a function GetCapability(Capability.Fill), which will return the fill for the items if it exists otherwise returns NULL. If some of your objects already implement a Fillable interface, then you can return the object cast to a Fillable pointer instead.

1
  • Unfortunately you then once again have a fat base class... This is quite unfortunate. Sep 7, 2011 at 17:13
0

Consider storing Variant items, such as boost::variant.

You can define a boost::variant<Fillable*,Item*> (you should use smart pointers if you have ownership), and then have a list of those variants on which to iterate.

2
  • Your fat class becomes the variant... I don't like this approach, especially if the number of fill-like interfaces is expected to grow. What I particularly dislike is that usually, you have Fillable inherit Item. You'll have to defend this one better :) Sep 7, 2011 at 17:28
  • @Alexandre C: The class itself was never thouched, and thus clients that do not require the Fillable property never see the fill method. The "fat" class is therefore local to the use of the "fat" interface, and all is well! (and you are also about as fast as the "fat" class was) Also, the mechanism can grow locally without requiring Item to implement a full blown Visitor interface. Sep 7, 2011 at 17:45
0

I suggest using an interface for the shapes, with a method that returns a filler. For example:

class IFiller {
public:
    virtual void Fill() = 0;

protected:
    IFiller() {}
    virtual ~IFiller() {}
};

class IShape {
public:
    virtual IFiller* GetFiller() = 0;

protected:
    IShape() {}
    virtual ~IShape() {}
};

class NullFiller : public IFiller {
public:
    void Fill() { /* Do nothing */ }
};

class Text : public IShape {
public:
    IFiller* GetFiller() { return new NullFiller(); }
};

class Rectangle;
class RectangleFiller : public IFiller {
public:
    RectangleFiller(Rectangle* rectangle) { _rectangle = rectangle; }
    ~RectangleFiller() {}

    void Fill() { /* Fill rectangle space */ }

private:
    Rectangle* _rectangle;
};

class Rectangle : IShape {
public:
    IFiller* GetFiller() { return new RectangleFiller(this); }
};

I find this method easier to maintain and to extend, while it does not introduce major performance issues.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.