I have a class that contains a private std::vector and a flag. Every time an Object inside the vector is modified I want to set that flag.

This is what I came up with..

struct Object
{
    float f;
    int i;
}

class SomeClass
{
private:
    std::vector<Object> Data;
    bool Updated;

public:
    inline const std::vector<Object>& ReadData const
    {
        return Data;
    }
    inline std::vector<Object>& WriteData
    {
        Updated = false;
        return Data;
    }
}

This design is the safest I could come up with. The only issue with this is the fact I can bypass this safety mechanism when I'm using Data inside SomeClass.

So, even though I don't have lots of methods in SomeClass that operate on Data, could you suggest me a better design to achieve this? (Tracking every change to Data by setting Updated accordingly)

CodePudding user response：

Your solution fails to achieve what you want.

SomeClass has an Update method that does some stuff and sets Updated to true. Data is used inside a couple of big functions, and first thing they do is to check Updated and call Update when it's false. Again this is just the best I could come up with and I always feel naked when showing my ideas to the experts here

I guess what you had in mind was something like this:

SomeClass sc;
auto& data = sc.WriteData(); 
sc.Update();                     // checks if Updated is set to false
                                 // if necessary Updates the data and
                                 // resets Updated to true
sc.do_some_thing_with_data();    // this can use Data as is because Updated is true

But your "encapsulation" does not prevent a user to do this:

SomeClass sc;
auto& data = sc.WriteData();  // Updated = false
sc.Update();                  // Updated = true

data[0] = 42;                 // modify Data

sc.do_some_thing_with_data(); // assumes data has not been modified 

I suppose you actually want to call Update inside do_some_thing_with_data, however that does not change the problem:

auto& data = sc.WriteData();  // Updated = false

sc.do_some_thing_with_data(); // checks Updated, updates the data
                              // and resets Updated = true
                              // assumes data is updated -> OK
data[0] = 42;

sc.do_some_thing_with_data(); // checks Updated,
                              // does not update data because Updated is still true
                              // assumes data is updated -> WRONG

Once you returned a non-const reference you are no longer in control of modifiying the member. Your flag does not provide the "security" you expect. Returning a non-const reference is not encapsulation!

You can either not return a non-const reference, or don't pretend to encapsulate data when you don't encapusalte data (ie make the member public). If you want to be in control of modifications to data then do something along the line of:

class SomeClass
{
private:
    std::vector<Object> Data;
    bool Updated;

public:
    const std::vector<Object>& ReadData const
    {
        return Data;
    }
    void WriteData(const Object& obj,size_t index)
    {
        Data[index] = obj;
        Updated = false;
    }
    void WriteF(float f,size_t index){
        Data[index].f = f;
        Updated = false;
    }
    void WriteI(int i,size_t index){
        Data[index].i = i;
        Updated = false;
    }
}

And if you are worried about SomeClass being able to modify Data by bypassing the setters then thats because you want SomeClass do too much. Do other stuff in a different class:

 struct SomeOtherClass {
       SomeClass sc;
 };

Now SomeOtherClass can only modify Data in a controlled way.

PS: You seem to got confused by encapsulation vs no encapsulation. To reiterate: Returning a non-const reference is not encapsulation. If you want to encapsulate Data then make it private and do not provide direct access to it. The class that is in charge of encapsulating data has direct access to it. Other classes don't.

CodePudding user response：

The only issue with this is the fact I can bypass this safety mechanism when I'm using Data inside SomeClass.

Sooooo encapsulate what you want to encapsulate, not something else. You could put your whole program in the class, and argue it would have access then to all private members. So don't, encapsulate it.

class Data {
   std::vector<Object> data;
   bool updated;
public:
   std::vector<Object>& read() const {
       return data;
   }
   std::vector<Object>& write() {
        updated = false;
        return Data;
   }
};


class SomeClass {
    Data data;
public:
    const std::vector<Object>& readData() const {
        return data.read();
    }
    std::vector<Object>& writeData() {
        return data.write();
    }
};