When should a private method take the public route to access private data?

Question

When should a private method take the public route to access private data? For example, if I had this immutable 'multiplier' class (a bit contrived, I know):

class Multiplier {
public:
    Multiplier(int a, int b) : a(a), b(b) { }
    int getA() const { return a; }
    int getB() const { return b; }
    int getProduct() const { /* ??? */ }
private:
    int a, b;
};

There are two ways I could implement getProduct:

    int getProduct() const { return a * b; }

or

    int getProduct() const { return getA() * getB(); }

Because the intention here is to use the value of a, i.e. to get a, using getA() to implement getProduct() seems cleaner to me. I would prefer to avoid using a unless I had to modify it. My concern is that I don't often see code written this way, in my experience a * b would be a more common implementation than getA() * getB().

Should private methods ever use the public way when they can access something directly?

Answer 1

It depends of the actual meaning of a, b and getProduct.

The purpose of getters is to be able to change the actual implementation while keeping the interface of the object the same. For example, if one day, getA becomes return a + 1;, the change is localized to a getter.

Real scenario cases are sometimes more complicated than a constant backing field assigned through a constructor associated with a getter. For instance, the value of the field may be computed or loaded from a database in the original version of the code. In the next version, caching may be added to optimize performance. If getProduct continues to use the computed version, it won't benefit from caching (or the maintainer will do the same change twice).

If it makes perfect sense for getProduct to use a and b directly, use them. Otherwise, use getters to prevent maintenance issues later.

Example where one would use getters:

class Product {
public:
    Product(ProductId id) : {
        price = Money.fromCents(
            data.findProductById(id).price,
            environment.currentCurrency
        )
    }

    Money getPrice() {
        return price;
    }

    Money getPriceWithRebate() {
        return getPrice().applyRebate(rebate); // ← Using a getter instead of a field.
    }
private:
    Money price;
}

While for the moment, the getter doesn't contain any business logic, it is not excluded that the logic in the constructor will be migrated to the getter in order to avoid doing database work when initializing the object:

class Product {
public:
    Product(ProductId id) : id(id) { }

    Money getPrice() {
        return Money.fromCents(
            data.findProductById(id).price,
            environment.currentCurrency
        )
    }

    Money getPriceWithRebate() {
        return getPrice().applyRebate(rebate);
    }
private:
    const ProductId id;
}

Later, caching may be added (in C#, one would use Lazy<T>, making the code short and easy; I don't know if there is an equivalent in C++):

class Product {
public:
    Product(ProductId id) : id(id) { }

    Money getPrice() {
        if (priceCache == NULL) {
            priceCache = Money.fromCents(
                data.findProductById(id).price,
                environment.currentCurrency
            )

        return priceCache;
    }

    Money getPriceWithRebate() {
        return getPrice().applyRebate(rebate);
    }
private:
    const ProductId id;
    Money priceCache;
}

Both changes were focused on the getter and the backing field, the remaining code being unaffected. If, instead, I had used a field instead of a getter in getPriceWithRebate, I would have to reflect the changes there as well.

Example where one would probably use private fields:

class Product {
public:
    Product(ProductId id) : id(id) { }
    ProductId getId() const { return id; }
    Money getPrice() {
        return Money.fromCents(
            data.findProductById(id).price, // ← Accessing `id` directly.
            environment.currentCurrency
        )
    }
private:
    const ProductId id;
}

The getter is straightforward: it is a direct representation of a constant (similar to C#'s readonly) field which is not expected to change in the future: chances are, ID getter will never become a computed value. So keep it simple, and access the field directly.

Another benefit is that the getId might be removed in the future if it appears that it is not used outside (like in the previous piece of code).

Answer 2

Typically, you would use the variables directly. You expect to change all members when changing a class's implementation. Not using the variables directly simply makes it more difficult to correctly isolate the code that depends on them and makes it harder to read the member.

This is of course different if the getters implement real logic, in that case it depends on whether you need to utilize their logic or not.

Answer 3

I would say that using the public methods would be preferable, if not for any other reason but to conform to DRY.

I know in your case, you have simple backing fields for your accessors, but you could have certain logic, e.g. lazy loading code, which you need to run before the first time you use that variable. Thus, you would want to call your accessors instead of directly referencing your fields. Even though you don't have this in this case, it makes sense to stick to a single convention. That way, if you ever make a change to your logic, you only have to change it in one place.

Answer 4

For a class this small, simplicity wins. I would just use a * b.

For something far more complicated, I would strongly consider using getA() * getB() if I wanted to clearly separate the "minimal" interface from all the other functions in the full public API. An excellent example would be std::string in C++. It has 103 member functions, but only 32 of them really need access to private members. If you had a class that complex, forcing all the "non-core" functions to consistently go through the "core API" could make the implementation a lot easier to test, debug, and refactor.