0

I have the following class:

    class Document
    {
    public:
        Document():
         // default values for members, 
         // ...
         m_dirty{false}{}

        // Accessor functions

        template<class OutputStream>
        Document& save(OutputStream stream)
        {
            // Write stuff to `stream`
            // ...

            m_dirty = false;
            return *this;
        }

        bool dirty() const { return m_dirty; }

    private:
        Size2d m_canvas_size;
        LayerStack m_layers;
        LayerIndex m_current_layer;

        std::vector<Palette> m_palettes;
        PaletteIndex m_current_palette;
        ColorIndex m_current_color;

        std::vector<std::string> m_palette_names;
        std::vector<std::string> m_layer_names;

        bool m_dirty;
    };

Should the class have public member functions for modifying an element of say m_palettes directly, like

Document& color(PaletteIndex, ColorIndex, Color)

, or is it more "correct", to only allow access to the entire vector, through a pair of API:s

std::vector<Palette> const& palettes();
Document& palettes(std::vector<Palette>&&);

The first option would be more efficient, since it would not require to create a temporary copy of the data member, but consistent use of this design would make the interface bloated. It would require "deep" getters and setters for every container in the class.

Notice the dirty flag. Thus, the following would break the abstraction:

std::vector<Palette>& palettes();
user877329
  • 6,717
  • 8
  • 46
  • 88
  • 1
    You might have Proxy to "propagate" dirty flag from `Palette` modification. – Jarod42 Jul 15 '20 at 18:26
  • @Jarod42 So basically an object with private references to the container and the dirty flag, and then the appropriate methods on that proxy. Sounds like a good approach. – user877329 Jul 15 '20 at 18:37
  • ... and the destructor of the proxy sets the dirty flag. But it has to have a name. – user877329 Jul 15 '20 at 18:44

2 Answers2

0

You might have Proxy to "propagate" dirty flag from Palette modification, something like:

template <typename T>
class DirtyProxy
{
    T& data;
    bool& dirty;
public:
    DirtyProxy(T& data, bool& dirty) : data(data), dirty(dirty) {}
    ~DirtyProxy() { dirty = true;}
    DirtyProxy(const DirtyProxy&) = delete;

    T* operator ->() { return data; }
};

And then

DirtyProxy<Palette> palette(std::size_t i) { return {m_palettes.at(i), dirty}; }
Jarod42
  • 203,559
  • 14
  • 181
  • 302
0

I think the most robust way to solve it is to use a a callback. An issue with the proxy is that it would not handle the case where the the client code throws an exception (assuming strong exception guarantee). Testcase:

try
{
    auto property_proxy = obj.getProperty();
    // an exception is thrown here...
    property_proxy->val = x;  // Never updated

}
catch(...)
{}

assert(!obj.dirty());

will fail, because the dtor always sets the dirty flag. However with a callback

class Foo
{
    public:
        template<class F>
        Foo& modifyInSitu(F&& f)
        {
            f(x);
            m_dirty = true;
            return *this
        }
};

will only update m_dirty, when f(x) does not throw.

user877329
  • 6,717
  • 8
  • 46
  • 88