When manipulating data members: which of the following is considered best practice

Question

I want to start adopting best practice and have seen class members being manipulated in different ways. I am not aware of any subtle nor significant differences in the following example.

I wish to clarify an optimal approach if any of the two or another suggestion.

const Fraction & Fraction::timesEq(const Fraction & f) {

  //First approach
  numerator *= f.numerator;
  denominator *= f.denominator;

  //Second approach
  numerator *= f.getNumerator();
  denominator *= f.getDenominator();

  return (*this); //would 'return' statement this be considered best practice?
}

Answer 1

The second approach survives subclassing and possible virtual redifinitions of the methods if this matters for the particular case but more cumbersome and boring.

Answer 2

In a simple class representing rational numbers such as yours, I would follow the KISS principle and go with the first.

If the class is more complex and/or you need the flexibility of (possibly virtual) getters/setters, it can be a good idea to be consistent and decouple from the representation completely:

const Fraction & Fraction::timesEq(const Fraction & f) {
    setNumerator(getNumerator() * f.getNumerator());
    setDenominator(getDenominator() * f.getDenominator());
    return *this;
}

Whether that's worth the added complexity needs to be decided case by case.

Answer 3

I would recommend a third approach. It insulates the function from the representation of the numerator and the denominator.

onst Fraction & Fraction::timesEq(const Fraction & f) {

  this->getNumerator() *= f.getNumerator();
  this->getDenominator() *= f.getDenominator();

  return (*this);
}