2

Say I have a class Foo with a vector_ data member like so:

class Foo {
public:
    const std::vector<int> & vector() const {
        return vector_;
    }
    void vector(const std::vector<int> &vector) {
        vector_ = vector;
        // Other operations which need to be done after the 
        // vector_ member has changed
    }
private:
    // Some large vector
    std::vector<int> vector_;
};

I often face situations like these

void someOperation(std::vector<int> &v) {
    // Operate on v, but almost always let v's size constant
}

int main() {
    // Create Foo object
    Foo foo;
    // Long loop
    for (auto k = 0; k < 100; k++) {
        auto v = foo.vector();
        someOperation(v);
        foo.vector(v);
    }
}

where I can't pass foo's (possibly large) vector_ member directly to someOperation due to the (const-correct) implementation of the vector method to access the member. Although someOperation almost always lets its argument's size unchanged, I need to copy the vector first, then pass it to someOperation and then to foo's setter. Clearly, I can avoid this extra copy if I remove the const-ness of the Foo's class getter and call an afterChange method after the member has been changed by someOperation - but this breaks encapsulation:

class Foo {
public:
    std::vector<int> & vector() { // Note we now return by non-const reference
        return vector_;
    }
    void afterChange() {
        // Other operations which need to be done after the 
        // vector_ member has changed
    }
private:
    std::vector<int> vector_;
};

Are there any other alternatives? Or is this one of the situations where breaking encapsulation is legitimate?

Marcel
  • 616
  • 1
  • 5
  • 15
  • 3
    It seems to me that maybe `someOperation` should be an operation on the `foo` object instead, i.e. it should be a member function in the `Foo` class. – Some programmer dude Apr 26 '16 at 04:48
  • It seems to me you don't really have any encapsulation to break if this vector is passed in and out of the class object for regular processing. – Galik Apr 26 '16 at 06:24
  • @Galik: I'm not sure if I got your point. If the member is modified from outside the class (e.g., by returning a non-`const` reference), I break encapsulation (also note that the setter may perform additional operations after the member has changed). I think the alternatives mentioned by @RichardHodges make changing the member more explicit and promote efficiency at the same time. – Marcel Apr 26 '16 at 06:51
  • Here is a Regex for Visual Studio which may help find cases of methods returning non-const references: `(?<!const\s+(?:\w+\s+)?|::)\b\w+(?:::\w+)*\s*(?:<[^<>]*>\s*)*&\s+\w+(?:::\w+)*\s*\(` — May be not perfect, someone can suggest improvements! – sergiol Apr 18 '18 at 13:07
  • Related: [Why do people write private-field getters returning a non-const reference?](https://stackoverflow.com/questions/42587067/why-do-people-write-private-field-getters-returning-a-non-const-reference) – sergiol Apr 20 '18 at 11:32
  • Related: [Does it make sense to provide non-const reference getter](https://stackoverflow.com/questions/4113845/does-it-make-sense-to-provide-non-const-reference-getter/4113864) – sergiol Apr 20 '18 at 11:33

2 Answers2

3

in your case you could gain some efficiency by moving the vector out of the class and back in again:

class Foo {
public:
    std::vector<int>&& take_vector() {
        return std::move(vector_);
    }

    void vector(std::vector<int> vector) {
        vector_ = std::move(vector);
        // Other operations which need to be done after the 
        // vector_ member has changed
    }
private:
    // Some large vector
    std::vector<int> vector_;
};

then...

void someOperation(std::vector<int> &v) {
    // Operate on v, but almost always let v's size constant
}

int main() {
    // Create Foo object
    Foo foo;
    // Long loop
    for (auto k = 0; k < 100; k++) {
        // this is a very cheap move
        auto v = foo.take_vector();

        someOperation(v);

        // so is this
        foo.vector(std::move(v));
    }
}

or you could structure the operation on the vector as a visitor:

class Foo {
public:
    template<class F>
    void apply_op(F&& op) {
        op(vector_);
        // Other operations which need to be done after the 
        // vector_ member has changed
    }
private:
    // Some large vector
    std::vector<int> vector_;
};

invoked like so:

void someOperation(std::vector<int> &v) {
    // Operate on v, but almost always let v's size constant
}

int main() {
    // Create Foo object
    Foo foo;
    // Long loop
    for (auto k = 0; k < 100; k++) 
    {
        foo.apply_op(&someOperation);
    }
}
Richard Hodges
  • 68,278
  • 7
  • 90
  • 142
0

In your case, you can just change someOperation() to work over a range, not the vector itself. Your Foo class would then need begin() and end() functions, returning appropriate iterators.

Nikos C.
  • 50,738
  • 9
  • 71
  • 96