Prevent breaking encapsulation

Question

I have this class:

class Phone {
private:
    string producer, color;
    int weight, dimension;
public:
    Phone(string &producer, string &color, int &weight, int &dimension):
        producer(producer), color(color), weight(weight), dimension(dimension) {};
    Phone():
        producer(""), color(""), weight(0), dimension(0) {};
    virtual ~Phone() {};
    string getProducer(void) const;
    string getColor(void) const;
    int getWeight(void) const;
    int getDimension(void) const;
    virtual void displayInfo(void) const;
};

The problem is here caused by the fact that I expose the internal implementation of the object via getters.

But how can I prevent this?

Because usually in my code, I need to know some private data from my object (for comparision is one example), and that's why I use getters.

So then I rewrite the class to something like this:

class Phone {
    private:
        string producer, color;
        int weight, dimension;
    public:
        Phone(string &producer, string &color, int &weight, int &dimension):
            producer(producer), color(color), weight(weight), dimension(dimension) {};
        Phone():
            producer(""), color(""), weight(0), dimension(0) {};
        virtual ~Phone() {};
        bool isTheProducer(string& producer) const { return this->producer == producer };
        bool hasWeight(int& weight) const { return this->weight == weight };
        bool hasDimension(int& dimension) const { return this->dimension == dimension };
        virtual void displayInfo(void) const;
    };

Is this a better design (by the fact that I don't get the actual private value)?

Answer 1

As you might have seen from the other answers and comments, the answer is: It depends.

In fact, it depends mainly on the usecases where your class is used. Let's stick first to the example given in the question, the comparison of objects. Since it is not clearly visible from the question if we want to compare two phone objects or just a specific data member I will discuss both situations here.

Comparing a data member to out-of-class data

Let's take this usecase where we search for all phones with a weight bigger than x(just pseudocode):

for (Phone& p in phoneList) {
    if (p.getWeight() > x) {
        cout << "Found";
    }
}

Then the first class example is perfectly fine, since this is not an intrinsic feature of the phone, and thus the phone class is not responsible for handling it. In addition, the result does not expose more than absolutely required for the task.

Comparing two phone objects

In this case both code examples are equally good (or in this case equally bad). In both cases the user has to know a lot of details about how phones are represented to compare all necessary members. If in a later revision a new member is added to the class, every code segment that compares two phones has to be adapted. To overcome this, one can add a function to the class that does exactly the comparison.

class Phone {
private:
    string producer, color;
    int weight, dimension;
public:
    bool IsEqualTo(const Phone& other)
    {
        return (producer == other.producer && color == other.color &&....);
    }

Non comparitive usecase

But let's go to a more advanced example. Let's assume the following task: A user enters the pin to a phone and if it is the correct one, the phone should unlock. Let's assume a very naive approach:

class Phone
{
private:
    int pin;
    bool unlocked;

public:
    int getPin() { return pin; }
    void unlock() { unlocked = true; }
};

and the corresponding call

if (phone.getPin() == enteredPin)
    phone.unlock();

In this case we have a totally different situation. Here we need to consider the "tell, don't ask" rule, which basically says that it is a bad design to query the state of an object first, make a decision and then tell the object what to do. Instead we should only tell the object what we want, and let it do the work for us. In this usecase this is obvious, since unlocking the phone only when the pin is correct is a responsibility of the phone, not of the user that uses the phone class. But in more complex scenarious many programmers will do exactly what I described here.

Back to the problem: A good solution here would be for example

class Phone
    {
    private:
        int pin;
        bool unlocked;

    public:
        void CheckPin(int enteredPin) {
            if (pin == enteredPin)
               unlocked = true;
        }
    };

with the code

phone.CheckPin(enteredPin);

Hope this helps, and thanks to @KonradRudolph for pointing to the "tell, don't ask rule". Feel free to help me to improve the answer per commenting on it :)