Pergunta

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;
}
Foi útil?

Solução

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.

Outras dicas

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.

Licenciado em: CC-BY-SA com atribuição
scroll top