I'm learning C and I need to know if it is a good idea to write member functions of a class in terms of private setters and getters, or should I work with raw private data?.

I have this class:

    class Complex{
    public:

        Complex();
        Complex(const double &real_part);
        Complex(const double &real_part, const double &imaginary_part);

        Complex& operator  = (const Complex &addend);
        Complex& operator *= (const Complex &multiplier);
        
        Complex operator ~ ();
        Complex operator - ();

        Complex& operator -= (Complex &subtrahend);
        Complex& operator /= (Complex &divisor);

    private:

        double real_part_, imaginary_part_;
        double re() const {return real_part_;};
        double im() const {return imaginary_part_;};
        void set_real_part(const double real_part){real_part_ = real_part;};
        void set_imaginary_part(const double imaginary_part){imaginary_part_ = imaginary_part;};
    

    friend double Re(Complex &z){return z.re();};
    friend double Im(Complex &z){return z.im();};
    };

    // Constructors

    Complex::Complex(){
        set_real_part(0);
        set_imaginary_part(0);
    }
    
    Complex::Complex(const double &real_part){
        set_real_part(real_part);
        set_imaginary_part(0);
    }
    Complex::Complex(const double &real_part, const double &imaginary_part){
        set_real_part(real_part);
        set_imaginary_part(imaginary_part);
    }
    
    /////////////////Members///////////////
    
    //Basic perators
    
    Complex &Complex::operator  =(const Complex &other){
        set_real_part(re()   other.re());
        set_imaginary_part(im()   other.im());
        return *this;
    }
    
    Complex &Complex::operator *=(const Complex &other){
        double real_part = re()*other.re() - im()*other.im();
        double imaginary_part = re()*other.im()   im()*other.re();
        set_real_part(real_part);
        set_imaginary_part(imaginary_part);
        return *this;
    }
    
    //Conjugate and inverse
    
    Complex Complex::operator ~(){
        Complex other(re(),-im());
        return other;
    }
    
    Complex Complex::operator -(){
        Complex other(-re(),-im());
        return other;
    }
    
    // Inverse operations
    
    Complex &Complex::operator -=(Complex &other){
        *this  = -other;
        return *this;
    }
    
    Complex &Complex::operator /=(Complex &other){
        *this *= ~other;
        other *= ~other;
        set_real_part(re()/other.re());
        set_imaginary_part(im()/other.re());
        return *this;
    }

Friend functions are for using getters as, for example, Re(z) instead of z.re(), so I can write an expression inside the function.

Also, if you can give a review, it will be very much appreciated.

CodePudding user response：

Anything that is marked private is only accessible to the class implementation. Therefore, you can (and should) remove these functions:

    double re() const {return real_part_;};
    double im() const {return imaginary_part_;};
    void set_real_part(const double real_part){real_part_ = real_part;};
    void set_imaginary_part(const double imaginary_part){imaginary_part_ = imaginary_part;};

Any code that could call these functions already has access to real_part_ and imaginary_part_ and could therefore use them directly.

As to your question about whether the syntax should be Re(im) or im.re(), this is largely a matter of personal preference. The syntax im.re() is more common in C codebases, though the advantage of having Re(im) mirror the math syntax is nice. I don't think either of these is clearly right or clearly wrong.

CodePudding user response：

Appealing to the core guidelines for this.

"Use class if the class has an invariant; use struct if the data members can vary independently."

In other words, if your data members have a constraint that they need to satisfy, make them private and expose functions to modify them to maintain this constraint. The example given here is a Date class:

class Date {
public:
    // validate that {yy, mm, dd} is a valid date and initialize
    Date(int yy, Month mm, char dd);
    // ...
private:
    int y;
    Month m;
    char d;    // day
};

You wouldn't want the user to be able to set the day to 100 as that's not a valid date, so make the members private and have a function called advance that moves the date forward 100 days, which would also update the month and possibly year stored in the object.

For something such as a complex number, the real and imaginary parts are independent of each other, so I would just use a plain struct and expose the values like so

struct Complex
{
    double real, imag;
};

Less boilerplate to maintain, and easier to read. This particular case also has the benefit that it's an aggregate type, so you don't need to define any constructors, you can just instantiate like

Complex z1{3, 4};
auto z2 = Complex{3, 4};
auto z3 = Complex{ .real = 3, .imag = 4}; // C  20, and the best in my opinion

You can still implement all the operators you want as free functions like

Complex operator (const Complex& lhs, const Complex& rhs) {
    return Complex{lhs.real   rhs.real, lhs.imag   rhs.imag};
}

double& real(const Complex& lhs) { return lhs.real; }

which can be free functions that don't require friendship to implement.

It's not always clear which case you want, however if you find yourself writing getters and setters for all data members, you may just want a struct.