Does changing the private member of a class through an overloaded subscript operator break encapsulation?

Question

I have always been taught that in order to in-keep with encapsulation objects should be responsible for performing calculations on their own data.

So in this case here is this breaking rules of encapsulation and considered bad practice?

#include <iostream>
using namespace std;
const int SIZE = 10;

class safearay {
private:
    int arr[SIZE];
public:
    safearay() {
        register int i;
        for (i = 0; i < SIZE; i++) {
            arr[i] = i;
        }
    }

    int &operator[](int i) {
        if (i > SIZE) {
            cout << "Index out of bounds" << endl;
            // return first element.
            return arr[0];
        }
        return arr[i];
    }
};

int main() {
    safearay A;

    cout << "Value of A[2] : " << A[2] << endl;
    cout << "Value of A[5] : " << A[5] << endl;
    cout << "Value of A[12] : " << A[12] << endl;
    A[5] = 2; // Does this break Encapsulation?? 
    cout << "Value of A[5] : " << A[5] << endl;
    return 0;
}

You're accessing a private member through an operator method. How is this different from accessing it through a normal method? Eventually you have to operate on private fields *somehow*, otherwise there would be no point in having them. — Kilian Foth, Feb 07 '17 at 08:49
operator methods and "normal" methods are both methods; neither is more or less encapsulated. — Frank Hileman, Feb 08 '17 at 22:48
Nobody noticed the bug... A [10] will return rubbish and possibly crash. — gnasher729, Feb 14 '17 at 13:19
If the index is out of bound, it should throw an exception. Returning another item might cause hard-to-find bugs. — Phil1970, Feb 15 '17 at 04:12
I does break the encapsulation so in general it is better to avoid that... except for case like this one where the purpose of the class is to represent an array. In those case, usually you would put the abstraction on the class that contains the array. — Phil1970, Feb 15 '17 at 04:18

score 2 · Accepted Answer · answered Feb 14 '17 at 04:59

The goal of encapsulation is to prevent arbitrary modifications of data. The most common way of achieving this in C++ is putting the data in the private section of a class and only putting a limited set of methods that may modify that data in the public section. Each public method tells the user what may be done to the data. The implementations of those methods have the responsibility of preserving the valid state of the data.

In your class, the operator[] allows any element of the array to be modified. If the user should be allowed to do this, then it's fine. It is your decision what the user is allowed to do. You even provide a safeguard to make sure that the index the user provides is within bounds. This is a good thing. This kind of checking is similar to the std::vector::at() method.

Some methods are harder to justify:

class Foo
{
    private:
        int x;
    public:
        int& my_x() { return x; }
}

With this class, the existence of the my_x() method is exactly the same as making int x public. This almost always represents broken encapsulation. It may be necessary, sometimes. The std::shared_ptr::get() method comes to mind, which exposes the pointer the shared_ptr is guarding. Sometimes, however, the user legitimately needs this kind of access. This is why the universal answer to all engineering questions is "It depends."

The problems with your class are inadequate safeguards in the array index checking. The user is not notified when the index i is negative. Also, printing to cout is not a strong enough warning to the user. Furthermore, returning a random element of the array when the index is out of bounds is user-hostile behavior. I say random because arr[0] is not the data the user asked for. Throwing an exception would be a better action to force the user to check their own code to get the right index.

I'd expect the flaws you see are owed to the desire to keep the example simple. From this view, the OP has put enough into the example to illustrate the point. — tofro, Feb 14 '17 at 06:36

gnasher729 · Answer 2 · 2017-02-14T14:55:10.073

1

You are breaking encapsulation in a bad way.

By returning a reference to an array element, you have lost control over that array element forever. I call

int* p = &A [5];

and now I can forever and at any time change the value of your private variable arr[5] (and actually the other elements as well, by writing p [-5] = p [4] = 0.

If you want to expose the array, expose the array. But you tried to hide it, and you didn't.

edited Feb 14 '17 at 14:55

answered Feb 14 '17 at 13:24

gnasher729

42,090
4
59
119

1

It is one of the main traits of an array that you can access its members using the array operator - *Not* returning a reference would be clashing with "normal" expectations and breaking an unwritten contract, IMHO. – tofro Feb 14 '17 at 14:16

Does changing the private member of a class through an overloaded subscript operator break encapsulation?

2 Answers2