2

I created a new class fraction and since it is common sense to have input for numerator and denominator. I was advised by my senior to declare numerator and denominator in private. But if they can be changed and data abuse can occur to them anyhow, then why should I declare them private?

My code for fraction class(if you want to refer to my code):

#ifndef FRACTION_H
#define FRACTION_H
#endif // FRACTION_H

#include<string>
#include<iostream>
using namespace std;

class fraction{
int numer, denom;
void change_to_simplest_form();
void get_value_in_string();
public:
    fraction(int,int);
    fraction(){numer =0; denom =0;}
    void display();
    int get_num(){return numer;}
    int get_denom(){return denom;}
    float get_float_num(){return float(numer);}
    float get_float_denom(){return float(denom);}
    void get_it_all_done();
    inline bool operator<(fraction &rhs){if(float(this->get_float_num()/this->get_float_denom())<float(rhs.get_float_num()/rhs.get_float_denom()))return true; else return false;}
    inline bool operator>(fraction &rhs){if(float(this->get_float_num()/this->get_float_denom())>float(rhs.get_float_num()/rhs.get_float_denom()))return true; else return false;}
    inline bool operator<=(fraction &rhs){if(float(this->get_float_num()/this->get_float_denom())<=float(rhs.get_float_num()/rhs.get_float_denom()))return true; else return false;}
    inline bool operator>=(fraction &rhs){if(float(this->get_float_num()/this->get_float_denom())>=float(rhs.get_float_num()/rhs.get_float_denom()))return true; else return false;}
    inline bool operator==(fraction &rhs){if(float(this->get_float_num()/this->get_float_denom())==float(rhs.get_float_num()/rhs.get_float_denom()))return true; else return false;}
    inline void operator++(int){numer+=denom;}
    inline void operator+=(int a){numer+=(denom*a);}
    inline void operator--(int){numer-=denom;}
    inline void operator-=(int a){numer-=(denom*a);}
    inline void operator=(string a){bool denom_not_one = true;int i =0;numer=0;denom=0;for(i =0;a[i]!='/';++i){if(a[i]=='\0'){denom=1;denom_not_one=false;break;}if(int(a[i])>=48 && int(a[i])<=57){numer*=10;if(a[i]!='0'){numer+=(int(a[i]-48));}}}for(;a[i]!='\0' && denom_not_one;++i){if(int(a[i])>=48 && int(a[i])<=57){denom*=10;if(a[i]!='0'){denom+=(int(a[i]-48));}}}}
    inline void operator=(fraction &rhs){this->numer=rhs.get_num();this->denom=rhs.get_denom();}
    inline fraction operator*(fraction &rhs){fraction tmp(this->numer*rhs.get_num(),this->denom*rhs.get_denom()); return tmp;}
    inline void operator*=(fraction &rhs){this->numer*=rhs.get_num();this->denom*=rhs.get_denom();}
    inline void operator/=(fraction &rhs){this->numer*=rhs.get_denom();this->denom*=rhs.get_num();}
};
void fraction:: get_it_all_done(){
change_to_simplest_form();
}

fraction::fraction(int a, int b){
    denom = b;
    numer =a;
//    display();
}

void fraction::change_to_simplest_form(){
int divisor = 1;
bool islessthanzero = false;
if(numer<0){
    numer*=(-1);
    islessthanzero = true;
}
while(divisor <= numer && divisor <= denom){
    if(numer%divisor==0){
        if(denom%divisor==0){
            numer/=divisor;
            denom/=divisor;
        }
    }

divisor++;
}
if(islessthanzero){numer*=(-1);}
}

void fraction::display(){
change_to_simplest_form();
cout << "(" << numer << "/" << denom << ")" ;
}



std::ostream &operator<<(std::ostream &os,fraction &m){
        m.get_it_all_done();

        if(m.get_denom()!=1){
            cout << "(" << m.get_num() << "/" << m.get_denom() << ")" ;

        }
        else cout << m.get_num();
            return os;
}
3
  • 1
  • 2
    Such noisy code. None of the casts to float is needed, nor are any of the this->s. I'd get rid of get_float_num() and get_float_denom(), and I'd write a single comparison function that looks at the integer values, and implement the comparison operators in terms of that function. Also, operator+=(int a) is rather peculiar, since it divides the value of the fraction by a ((with redundant parentheses)). Apr 13, 2016 at 14:45
  • Sometimes you want to run some extra code on "get" or "set". For example, you could cache some values, check access rights, and so on. Take a look at how properties are used in C# too.
    – OMGtechy
    Apr 13, 2016 at 14:47

6 Answers 6

7

This is the idea and principle of private data - public interface, whereby the public method used to change any private fields has suitable and appropriate defenses against altering the private fields to an unwanted value in the program (i.e. to avoiding the breaking of any invariants). If the field was simply public then there would be no such defense against this.

3
  • 1
    I would add the the technical term for 'preserving from unwanted value' is an keeping invariant.
    – SergeyA
    Apr 13, 2016 at 14:45
  • Oh. Now I got it... So, that no one simply changes private member to any inappropriate value such as 0/0. If I pass them through methods, I can check whether input is acceptable or not...? Am I right? By the way, can you give me a point (+1) for the question? Apr 13, 2016 at 14:51
  • @AyushMahajan Yes that's correct, and if the input is not acceptable then you can throw an exception in your code for example. Apr 13, 2016 at 14:57
1

Indeed, making members private and having public getters and setters breaks encapsulation, so from a design point-of-view it's all the same.

The only advantage is being able to set breakpoints in the getters and setters, which can help during debugging.

9
  • I don't think I agree with it breaking encapsulation. OP returns members by value, which doesn't allow a client to directly modify them. And I don't agree with setting breakpoints being the only advantage. I mean, with public getters you control how your data is accessed, allowing you to perform any number of checks on the data before returning. Or, you could compute the data on the fly. Or, you could implement a notification construct that lets other objects know the value has changed.
    – AndyG
    Apr 13, 2016 at 14:43
  • @AndyG I wouldn't call those getters and setters :). If you do any extra logic, where do you draw the line? How much validation until that setter is more than a setter? In a perfect (sometimes impossible) design, classes are self-contained, and if you need to access the internals of that class, something is wrong. Apr 13, 2016 at 14:45
  • I suppose something is wrong if you are not doing OOP. I think it's reasonable to want to set the name of a Person object.
    – AndyG
    Apr 13, 2016 at 14:47
  • @AndyG Should anyone be able to change the Person's name? (i.e. would that setter be public?) Apr 13, 2016 at 14:49
  • I agree with Luchian. if the setter (in particular) is doing nothing more than setting, then you may as well have data-is-interface semantics (e.g. std::pair). If the object requires intelligence (e.g. range checking) then a getter/setter becomes appropriate. Apr 13, 2016 at 14:50
1

To isolate changes (from implementation).

For your case, if you want to change numerator and denominator's type from int to float, if they're public without getter, all the clients need to change their code too. If they're private with public getter, the clients don't need to change since the interface doesn't change. It makes you more flexible to change the implementation details.

1

You may refer your professor to this answer.

In many cases, where a class is merely a convenient placeholder for discrete values, then it is appropriate for the data to be the interface.

An example of this is std::pair which has the public data members first and second. It is entirely appropriate to modify one without referring to the other, so there is no sense in cluttering up the code with get_first() and set_first() (note that the free functions auto& std::get<0>(t) and auto& std::get<1>(t) mimic this behaviour, but for a specific reason - they make generic programming easier).

Another time data-is-interface is more than appropriate is when the data member is const. By being const, it cannot change so there's no need for the idea of get/set (it's also thread-safe, but that's another story).

so you could implement your fraction as:

struct frac
{
  double numerator, denominator;
};

and allow changes to the arguments.

But actually in a fraction, the numerator and denominator are not independent. For example, setting a denominator of zero should probably be caught early (by throwing an exception!) and you may want to normalise the fraction at the earliest opportunity, depending on your strategy for implementing fractional arithmetic.

so this starts to become more sensible:

struct frac
{
  frac(double n, double d) : numerator(n), denominator(d) {}

  frac& set_denominator(double d)
  {
    if (d == 0.0) {  // probably should check the ratio of n to d here
                     // but this is a simplified example for exposition
      throw std::invalid_argument(std::to_string(d));
    }
    denominator = d;
    normalise();
    return *this;
  }

  void normalise() {
    // find gcd etc etc
  }

private:
  double numerator, denominator;
};

Another approach is to design your fraction to be immutable, and demand that any change results in a copy.

In this case, this is appropriate:

struct frac
{
  frac(double n, double d) : numerator(n), denominator(d) {}

  frac replace_denominator(double d)
  {
    if (d == 0.0) {  // probably should check the ratio of n to d here
                     // but this is a simplified example for exposition
      throw std::invalid_argument(std::to_string(d));
    }
    return normalised_frac(numerator, d);
  }

  const double numerator, denominator;
};

frac normalised_frac(double n, double d)
{
  // normalisation operations on n and d
  return frac { n, d };
}

The case for each form will vary in strength depending on your requirements. In multi-processing, having immutable discrete copies is often preferred. It's the norm in languages such as Haskell and D.

1
  • Upvoted for the actual concise example aligning with question.
    – SergeyA
    Apr 13, 2016 at 15:11
0

This has to do with Encapsulation. It may seem like a waste of time if you are only using a basic setter to manipulate it, thus your question of why even make it private if you can still modify it. However, as your code becomes more intricate you will have other methods that will modify the private variables according to business logic.

Take, for example, a private variable weeklyEmployeePay, you may write a public method called calculateWeeklyEmployeePay(double hours, double rate). Then this public method will change the private variable based on the logic in the method and the given arguments.

Encapsulation is a key component to Object Oriented Programming.

0

In order to prevent data abuse. When the only access to them is provided by methods you write, your methods can be written in such a way as to prevent the private variables from getting bad values. If a caller attempts to set a value that would be inappropriate, you could do various things including:
- Throw an exception
- Clamp the value to a range that makes sense
- Display an alert. It's up to you.

So although a caller can send an inappropriate value to a public method, you have the option of doing something about it.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.