John Olivas
John Olivas

Reputation: 329

Why is it supposedly bad practice to declare a variable in the private part of a class? C++

I'm taking advanced C++ this semester in College, and I keep running into needing my work re-submitted because I'm using bad practice of declaring a variable without using mutator methods to set them.

I'm told that using code like this is bad practice.

class test{
private:
   int number = 5;
   int number2 = 10;
public:
   int addNums(){
       number3 = number+number2;
       return number3;}
};

My instructor recommends I should use declaration like this.

class test{
private:
   int number, number2;
public:
   void setNums(userInput, userInput2){//Where user input is gained from a seperate function setting as 5, 10.
       number = userInput;
       number2 = userInput2;}
   int addNums(){
       number3 = number+number2;
       return number3;}
};

Why is it bad practice to have a pre-defined variable when the program doesn't require user input? I mean, I can understand doing that if the user needs to put a number for the items, and then I would have done just this, but if it doesn't ask for user input, I'm not going to have user input.

Upvotes: 1

Views: 284

Answers (4)

Pete Becker
Pete Becker

Reputation: 76305

The fundamental design question you have to ask yourself is "what is the purpose of this class"?

As written, it has a member function named addNums() that always returns the value 15 (assuming that number3 has been declared somewhere. If that's its purpose, there's far too much mechanism; the member function should simply return 15, and if that's all it does, it's name should be changed because it doesn't add any numbers.

On the other hand, if the purpose of the class is to hold two values and return their sum, there has to be some way to set the values, either through the constructor or through some sort of member function. Once there's a way to set the values, addNums() does something useful, as its name suggests.

Upvotes: 0

Jens
Jens

Reputation: 9406

I would argue that setters are often a sign of bad design because they enable decisions about the objects state being made outside the class. In your (too) simple example, I would initialize the variables in the constructor:

class test{
private:
   int number;
   int number2;
public:
   test(int x, int y):
      number(x), number2(y) {}

   test(): test(5, 10) {}

   int addNums(){
       return number+number2;
   }
};

However, nobody knows what the test class should do or what responsibility it has. It may be better to change addNums to take two parameters, and remove the members completely.

Upvotes: 1

user743382
user743382

Reputation:

Your instructor is telling you to take just that minimal investment to making your class significantly more usable.

I mean, I can understand doing that if the user needs to put a number for the items, and then I would have done just this, but if it doesn't ask for user input, I'm not going to have user input.

And here you acknowledge that it would have the desired effect.

The question to ask yourself when designing is "what would my users expect?" Would reasonable users of that class expect that there is no way to pass in numbers? I think your instructor's answer to that is "no". I'm leaning that way as well.

Note that hardcoded inputs in your program are okay by the same logic. Would reasonable users of your program expect that there is no way to pass in numbers? Yes, that is exactly what the user asked for.

Upvotes: 2

hauron
hauron

Reputation: 4668

Does the class make any sense, if no user input is provided?

If yes, the pre-defined values may make sense as well. Although as it stands they're just magic numbers chosen because.

If not, the class object should not be creatable at all. Make sure the only way to construct is via a ctor, requiring the values.

If you wish, you may add default values to the ctor as well, although that is mixing the two approaches, and in fact is similar to default values provided at declarations.

Upvotes: 0

Related Questions