Should I use public or private variables?
Asked Answered
T

9

41

I am doing a large project for the first time. I have lots of classes and some of them have public variables, some have private variables with setter and getter methods and same have both types.

I decided to rewrite this code to use primarily only one type. But I don't know which I should use (variables which are used only for methods in the same object are always private and are not subject of this question).

I know the theory what public and private means, but what is used in the real world and why?

Timothytimour answered 18/1, 2013 at 13:26 Comment(4)
The OO answer would be to use private variables. But the visibility model in C++ is quite broken (a bit less so in C++11) and private members can cause very surprising issues if you don't give them uglified names.Oilstone
@MarcGlisse Wut?Junket
Nearly an exact dupe: #1596932Himes
Before C++11, having an overload using T::x in its signature would cause a hard error if you called another overload on a type with a private x member. But even in C++11, you still get issues like: gcc.gnu.org/bugzilla/show_bug.cgi?id=55713 where instead of ignoring private members (or bases) the compiler insists on giving an error. There are certainly worse examples. Several committee members have been heard calling the C++ access control broken though I think it might have been for different reasons.Oilstone
F
45

private data members are generally considered good because they provide encapsulation.

Providing getters and setters for them breaks that encapsulation, but it's still better than public data members because there's only once access point to that data.

You'll notice this during debugging. If it's private, you know you can only modify the variable inside the class. If it's public, you'll have to search the whole code-base for where it might be modified.

As much as possible, ban getters/setters and make properties private. This follows the principle of information hiding - you shouldn't care about what properties a class has. It should be self-contained. Of course, in practice this isn't feasible, and if it is, a design that follows this will be more cluttered and harder to maintain than one that doesn't.

This is of course a rule of thumb - for example, I'd just use a struct (equivalent with a class with public access) for, say, a simple point class:

struct Point2D
{
   double x;
   double y;
};
Forejudge answered 18/1, 2013 at 13:28 Comment(9)
I'd say the setter/getter creates an additional access point besides accessing the variable directly.Linhliniment
@Linhliniment but the direct access is limited to the class, so it's something. (if that's what you meant, than yes)Forejudge
+1 for "As much as possible, ban getters/setters." I always claim setters are evil (the functions, I mean. I don't mind the dogs).Boracite
@LuchianGrigore: All the same access is provided to the public anyway, so effectively you only created two ways of using the variable from inside the class... I don't really think restricting one of the access methods to the class helps if you in the same turn recreate a way to get all the same access from the outside anyway.Linhliniment
You should also mention that private members with public setters should be prefered to public members, because that's the only way you have control on how the member get's changed. This can (for example) be usefull to support upwards compatibility. Imagine that a new version of your Point2D only should handle values between -100.0 and +100.0. There is no (simple) way to implement this with the structure above without resulting in a code change of the clients using this structure.Bartle
@Aschratt that's not a Point2D anymore, is it. It's a RestrictedPoint2D. :)Forejudge
@LuchianGrigore Implying a subclass of Point2D that would then attempt to change the interface for Point2D. You never know how your classes are going to be used in the wild, using getters/setters provides a way to implement new functionality without breaking interface.Dourine
@Chris it was a joke. It's a trade-off - you're never sure nothing changes.Forejudge
I think getters are okay as long as it makes sense for that information to be visible. And I think setters are okay as long as they enforce the contract implied by the interface. By being a private field it is not automatically an implementation detail. We should be careful about giving people black and white answers. I think it is clear though that the common Java style of getters and setters for everything is probably bad.Parakeet
E
19

Since you say that you know the theory, and other answers have dug into the meaning of public/private, getters and setters, I'd like to focus myself on the why of using accessors instead of creating public attributes (member data in C++).

Imagine that you have a class Truck in a logistic project:

class Truck {
public:
    double capacity;

    // lots of more things...
};

Provided you are northamerican, you'll probably use gallons in order to represent the capacity of your trucks. Imagine that your project is finished, it works perfectly, though many direct uses of Truck::capacity are done. Actually, your project becomes a success, so some european firm asks you to adapt your project to them; unfortunately, the project should use the metric system now, so litres instead of gallons should be employed for capacity.

Now, this could be a mess. Of course, one possibility would be to prepare a codebase only for North America, and a codebase only for Europe. But this means that bug fixes should be applied in two different code sources, and that is decided to be unfeasible.

The solution is to create a configuration possibility in your project. The user should be able to set gallons or litres, instead of that being a fixed, hardwired choice of gallons.

With the approach seen above, this will mean a lot of work, you will have to track down all uses of Truck::capacity, and decide what to do with them. This will probably mean to modify files along the whole codebase. Let's suppose, as an alternative, that you decided a more theoretic approach.

class Truck {
public:
    double getCapacity() const
        { return capacity; }

    // lots of more things...
private:
    double capacity;
};

A possible, alternative change involves no modification to the interface of the class:

class Truck {
public:
    double getCapacity() const
        { if ( Configuration::Measure == Gallons ) {
            return capacity;
          } else {
             return ( capacity * 3.78 );
          }
        }


    // lots of more things...
private:
    double capacity;
};

(Please take int account that there are lots of ways for doing this, that one is only one possibility, and this is only an example)

You'll have to create the global utility class configuration (but you had to do it anyway), and add an include in truck.h for configuration.h, but these are all local changes, the remaining of your codebase stays unchanged, thus avoiding potential bugs.

Finally, you also state that you are working now in a big project, which I think it is the kind of field in which these reasons actually make more sense. Remember that the objective to keep in mind while working in large projects is to create maintainable code, i.e., code that you can correct and extend with new functionalities. You can forget about getters and setters in personal, small projects, though I'd try to make myself used to them.

Hope this helps.

Eventempered answered 18/1, 2013 at 13:51 Comment(1)
I know this is a toy example, and I mostly agree with the conclusions, but please, if you're running a project including multiple units, don't do that. Instead, store everything using SI units internally, and only convert between units when interacting with the user (in the view layer if you're doing something remotely MVC-ish).Encyclopedic
K
12

There is no hard rule as to what should be private/public or protected.

It depends on the role of your class and what it offers.

  • All the methods and members that constitute the internal workings of the class should be made private.
  • Everything that a class offers to the outside world should be public.
  • Members and methods that may have to be extended in a specialization of this class, could be declared as protected.
Kerekes answered 18/1, 2013 at 13:50 Comment(0)
L
5

From an OOP point of view getters/setters help with encapsulation and should therefore always be used. When you call a getter/setter the class can do whatever it wants behind the scenes and the internals of the class are not exposed to the outside.

On the other hand, from a C++ point of view, it can also be a disadvantage if the class does lots of unexpected things when you just want to get/set a value. People like to know if some access results in huge overhead or is simple and efficient. When you access a public variable you know exactly what you get, when you use a getter/setter you have no idea.

Especially if you only do a small project, spending your time writing getters/setters and adjusting them all accordingly when you decide to change your variable name/type/... produces lots of busywork for little gain. You'd better spend that time writing code that does something useful.

C++ code commonly doesn't use getters/setters when they don't provide real gain. If you design a 1,000,000-line project with lots of modules that have to be as independent as possible it might make sense, but for most normal-sized code you write day to day they are overkill.

Linhliniment answered 18/1, 2013 at 14:8 Comment(0)
M
3

There are some data types whose sole purpose is to hold well-specified data. These can typically be written as structs with public data members. Aside from that, a class should define an abstraction. Public variables or trivial setters and getters suggest that the design hasn't been thought through sufficiently, resulting in an agglomeration of weak abstractions that don't abstract much of anything. Instead of thinking about data, think about behavior: this class should do X, Y, and Z. From there, decide what internal data is needed to support the desired behavior. That's not easy at first, but keep reminding yourself that it's behavior that matters, not data.

Mammiemammiferous answered 18/1, 2013 at 14:0 Comment(0)
B
1

Private member variables are preferred over public member variables, mainly for the reasons stated above (encapsulation, well-specified data, etc..). They also provide some data protection as well, since it guarantees that no outside entity can alter the member variable without going through the proper channel of a setter if need be.

Another benefit of getters and setters is that if you are using an IDE (like Eclipse or Netbeans), you can use the IDE's functionality to search for every place in the codebase where the function is called. They provide visibility as to where a piece of data in that particular class is being used or modified. Also, you can easily make the access to the member variables thread safe by having an internal mutex. The getter/setter functions would grab this mutex before accessing or modifying the variable.

I'm a proponent of abstraction to the point where it is still useful. Abstraction for the sake of abstraction usually results in a cluttered mess that is more complicated than its worth.

Brazil answered 18/1, 2013 at 16:36 Comment(0)
Z
1

I've worked with complex rpgies and many games and i started to follow this rule of thumb.

Everything is public until a modification from outside can break something inside, then it should be encapsulated.(corner count in a triangle class for example)

I know info hiding principles etc but really don't follow that.

Zillah answered 12/3, 2022 at 8:36 Comment(1)
I agree with this. If you strictly follow OOP then sure, you shouldnt manually be able to fiddle with the variables in a class. But we shouldnt just strictly follow something for no reason, we should make code as readable and maintainable as possible. If you just want a data structure to hold common elements together, there is no reason to make a class and then make a bunch of getters and setters. You should assume the programmer isnt an idiot/malicious and allow them access, unless there is a reason otherwiseInquisitionist
C
0

Public variables are usually discouraged, and the better form is to make all variables private and access them with getters and setters:

private int var;

public int getVar() {
  return var;
}

public void setVar(int _var) {
  var = _var;
}

Modern IDEs like Eclipse and others help you doing this by providing features like "Implement Getters and Setters" and "Encapsulate Field" (which replaces all direct acccesses of variables with the corresponding getter and setter calls).

Crissie answered 18/1, 2013 at 13:29 Comment(2)
Why is it better to make variables private and provide getters and setters? Calling that process "Encapsulate Field" is misleading; it's just replacing one syntax for access with a more verbose syntax for access. (Note, question is tagged C++.)Nonstriated
getters and setters offer a false sense of encapsulation.Forejudge
T
0

Hello I am working many years as a developer and therefore I always strictly used the OOP paradigm, but today I come to the finding that not always the encapsulation of private members is benifical. Currently I am creating a Diret2D Library where I need fast access to the members of different classes, and getting this data with accessors (getters) has always significant usage of stack and all access at least is established with a call.

Code snippet with getters:

   122:     fDrawingRect.left = GetControlRect().left;
00007FF723B886EF  lea         rdx,[rbp+4F8h]  
00007FF723B886F6  mov         rcx,qword ptr [this]  
00007FF723B886FD  call        ESC::CDXBaseControl::GetControlRect (07FF723B5CD89h)  
00007FF723B88702  cvtsi2ss    xmm0,dword ptr [rax]  
00007FF723B88706  movss       dword ptr [fDrawingRect],xmm0  
   123:     fDrawingRect.top = GetControlRect().top;
00007FF723B8870B  lea         rdx,[rbp+528h]  
00007FF723B88712  mov         rcx,qword ptr [this]  
00007FF723B88719  call        ESC::CDXBaseControl::GetControlRect (07FF723B5CD89h)  
00007FF723B8871E  cvtsi2ss    xmm0,dword ptr [rax+4]  
00007FF723B88723  movss       dword ptr [rbp+2Ch],xmm0  
   124:     fDrawingRect.right = GetControlRect().right;
00007FF723B88728  lea         rdx,[rbp+558h]  
00007FF723B8872F  mov         rcx,qword ptr [this]  
00007FF723B88736  call        ESC::CDXBaseControl::GetControlRect (07FF723B5CD89h)  
00007FF723B8873B  cvtsi2ss    xmm0,dword ptr [rax+8]  
00007FF723B88740  movss       dword ptr [rbp+30h],xmm0  
   125:     fDrawingRect.bottom = GetControlRect().bottom;
00007FF723B88745  lea         rdx,[rbp+588h]  
00007FF723B8874C  mov         rcx,qword ptr [this]  
00007FF723B88753  call        ESC::CDXBaseControl::GetControlRect (07FF723B5CD89h)  
00007FF723B88758  cvtsi2ss    xmm0,dword ptr [rax+0Ch]  
00007FF723B8875D  movss       dword ptr [rbp+34h],xmm0  

as you can see there are at least 4 call instructions which are really expensive. Consider this code must be accessed many times per second (thread based). So therefore I decided for my self that in case of GUI I will use public data members.

I just modifed the sourcecode to public members: (RECT)

   129:     fDrawingRect.left = m_ControlRect.left;
00007FF6CF7C86EF  mov         rax,qword ptr [this]  
00007FF6CF7C86F6  cvtsi2ss    xmm0,dword ptr [rax+8Ch]  
00007FF6CF7C86FE  movss       dword ptr [fDrawingRect],xmm0  
   130:     fDrawingRect.top = m_ControlRect.top;
00007FF6CF7C8703  mov         rax,qword ptr [this]  
00007FF6CF7C870A  cvtsi2ss    xmm0,dword ptr [rax+90h]  
00007FF6CF7C8712  movss       dword ptr [rbp+2Ch],xmm0  
   131:     fDrawingRect.right = m_ControlRect.right;
00007FF6CF7C8717  mov         rax,qword ptr [this]  
00007FF6CF7C871E  cvtsi2ss    xmm0,dword ptr [rax+94h]  
00007FF6CF7C8726  movss       dword ptr [rbp+30h],xmm0  
   132:     fDrawingRect.bottom = m_ControlRect.bottom;
00007FF6CF7C872B  mov         rax,qword ptr [this]  
00007FF6CF7C8732  cvtsi2ss    xmm0,dword ptr [rax+98h]  
00007FF6CF7C873A  movss       dword ptr [rbp+34h],xmm0  

as you can see there isn't any call instruction anymore and also no preparing for VTBL (rdx and rcx) as in the previous code. In best case when the 2 variables ( fDrawingRect and m_ControlRect) are the same type only two instructions will be used (mov rax,qword ptr.... and mov dword ptr [rbp...], rax.

Toby answered 15/7, 2024 at 23:27 Comment(0)

© 2022 - 2025 — McMap. All rights reserved.