Archived

This topic is now archived and is closed to further replies.

Turtlebread

Quick OOP question

Recommended Posts

I was writing some classes when I came up with this question. Say I have a class with a member variable named a. Is it worth it for one to code a like this: class MyClass{ public: void setA(int value); int getA(); private: int a; } And then access "a" through the functions. Or should I just make a public and access it through MyClass.a? I guess this is an opinion question, but I''m wondering what people think. -Chris

Share this post


Link to post
Share on other sites
Its really up to you.

The strict OOP guys will tell you its always best to use setters and getters in case you want to change how the class works and you want the setting of variable to kick off some other bit of code that you hadn''t forseen previously.. Abstracting the implementation and all that. In the real world, there''s plenty of cases where a variable is just a variable and you''ll never need to do that.

Unless you have to follow some coding guidelines for a job or for school, use whatever style you want... And, for the record, most sane compilers will optimize out the function call overhead by inlining if all the function is doing is setting a single variable based on a parameter, so in terms of run-time efficiency it doesn''t really matter which you use.

Share this post


Link to post
Share on other sites
Convention decrees that if it has public data members, it is a struct. Classes should not have public data members.

Share this post


Link to post
Share on other sites
the strict OOP guys do have a reason why you use getters and setters ... but they really only matter in 3 cases - the two cases when structs are inappropriate - here they are:

1. You want to impose restrictions on the values of the variable, or keep it''s changes synced with other values.
2. You want to perform some additional action when the value is changed or accessed.
3. You want to allow derived classes to change the behavior of the getters and setters.

Items 1 and 2 are really the key to deciding if you have a simple struct or if you are dealing with a more complex class. While it is acceptable to mix and match ... say have a class that has complex logic for 3 members and no logic for 2 other members ... it''s usually best to stay at the same level of abstraction ... presenting your users with a consistent interface.

Item 3 requires that you not only use getters and setters, but that you also declare them virtual ... but it should be fairly obvious when you want to do this.

NOTE: Using getters and setters can also allow for some simple debugging tricks ... like launching the debugger when a value is set to NULL .... or negative .. or whatever you don''t think you want to be doing.

Don''t try to avoid structs though ... they are immensly usefull for wrapping groups of data in easy to handle chunks - a struct with usefull constructors and/or operators can simplify your code a lot.

Good Luck

Share this post


Link to post
Share on other sites
The key difference being, of course, that structs are passed around, and information usually ends up being hidden anyway. Don''t hardcode value changes...it''s going to crash on your head, especially if you need to substitute a different implementation later.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
it isn''t really that difficult of an issue, the answer is: make it public unless there is a reason to make it private. If there is a reason, any reason at all no matter how small, then make it private.

For example we want to make a vector class. We can do it several different ways. We can store the x and y, we can store the angle and magnitude, or we can store both sets of information and keep them synchronized. Which one you use varies on what you are doing, most people use the x,y but there are reasons you might want to consider using the other ones. Now this looks like an argument for getters and setters but it isn''t. I think we can assume that you''ll make a permanent decision on which type you are using early in the project. For the first two types you''ll want to make everything public, there is absolutely no reason to make anything private. For the second one we have to make things private. The reason is that if they change any variable two others are supposed to change with it. So if we change magnitude the angle will stay the same but the x and y will change, likewise if we change x both magnitude and angle will change. Thus everything must be private.

Basically if variable B is dependant on A, then A must be made private and if B is totally dependant on A then B must be private as well. Making things private and using setters is the best way to keep states consistant. In my experience the whole "freedom to change representation" argument is rather very very overrated but valid. Sometimes you''ll want to make something private so you can change it later, but I find it to be one of the rarer uses of private members. Oh and one last piece of advice: learn all about various programming paradigms but don''t make any attempt to follow them. When it is a given paradigm''s turn to shine it will leap to your fingertips and apply itself. If you have to force a paradigm out then you probably aren''t using the right one.

As to the whole struct vs class distinction that''s just terminology. As far as I know C++ treats them identically. I just always use the class keyword because I am used to java.

Share this post


Link to post
Share on other sites
My answer is to taliesin73, although it is also valuable to the original poster as well:

A class should have no public data and only access them through accessor functions, as each object needs to be able to know when its state is altered.

It''s a simple philosophy, and one that saves you a lot of time fixing up code later in development. As an example... imagine a Document class for a text editor, with a Text object inside. (It could be a string, or list of strings, whatever). You might let your program alter the Text member directly, to add, delete, or replace words.

Then, at some point, you decide you want to add a ''modified'' flag to a Document, so that the system knows whether it needs saving to disk or not. If you used public member access to modify ''Data'', then you have to find all those places in the other code, and add a line like ''Document.SetAsModified()'' everywhere. But if you''d used accessor methods to alter Text, you would only need to add ''SetAsModified()'' in those methods.

Another example of a side-effect you might want, is a timestamp to mark the last time that you changed the object. Or the ability to add the change to an ''Undo'' list so that the changes can easily be undone later.

By using getter and setter routines, you are funnelling all access to a variable through a well-defined piece of code. As long as all you''re doing is modifying the variable directly, the compiler will optimise most of this out. And as soon as you want to do something more complicated than simply change the data type, you know you only have a minimal number of places where you need to change the code.

(As if that wasn''t important enough, there are the other added benefits of data and functional abstraction, the ability for derived classes to override the base class''s getter/setter functions, and so on.)

Share this post


Link to post
Share on other sites
Well, this is an opinion but I think that religiously making all members private and using Get() and Set() routines is not good programming. For something like a vector, where you are likely to want to reference the variables a lot, use of Get() and Set() are a pain in the arse. You should make your members private to get the "black box" effect - gratuitious use of Get() and Set() just leads to sloppy encapsulation, and overcomplicated, bloated classes.

Share this post


Link to post
Share on other sites
quote:
Original post by Sandman
Well, this is an opinion but I think that religiously making all members private and using Get() and Set() routines is not good programming.

I disagree on a technicality: using Get() and Set() to alter a variable is good programming, but making a class where it''s just a load of private variables with a Get and a Set for each one is a symptom of bad design. Rarely do all the variables in any single subsystem have to be exposed to the ''outside world''. Religiously making variables private is a good thing in terms of encapsulation, abstraction, and object-orientation. Adding a get and set for each one shows that you''re not exactly clear about your abstraction, or the boundaries of the object. Most of the time, you don''t need a ''Set'' for your internal variables, and quite a lot of the time, you don''t even need a ''Get'' either.

quote:
For something like a vector, where you are likely to want to reference the variables a lot, use of Get() and Set() are a pain in the arse.

If you mean a vector in the coordinate/positioning sense, then yes, there''s little reason not to use a struct/public members, since you tend to have to be acutely aware of the datatype you''re dealing with anyway. But I think most of the posts here are addressing slightly higher-level data types.

Share this post


Link to post
Share on other sites
It depends on what you do with it…

Get/Set tuples are generally bad for two reasons.
1) It doesn't protect the variable, thus destroying any illusion of encapsulation.
2) Get/Set methods that perform task can create side-effects that are highly undesirable. If twiddling a control from code sinks an event, you can end up with ugly loops and crappy boolean state variables. Imagine if UpdateData(FALSE) caused each UI object to execute it's OnChange event... this is the sort of thing that happens in VB and is caused by the set/get side-effects.

The object models of the various Office components workout quite well (better than the VB(A) visual objects). You can see how they did all the get/puts. If you do implement this sort of thing, you might want to look into COM, and the built-in support for these constructs.

Drop this in a C++ project (if you have Office 2000 installed) to see the Office interface in C++/COM code:

#import "C:\Program Files\Common Files\designer\MSADDNDR.TLB" raw_interfaces_only named_guids

#pragma warning(disable:4146) //signed enum stupidity
#import "C:\Program Files\Microsoft Office\Office\MSO9.DLL" //named_guids conflicts with the Excel import (duplicates)

#import "C:\Program Files\Common Files\Microsoft Shared\VBA\VBA6\VBE6EXT.OLB" named_guids

#undef DialogBox
#undef RGB
#import "C:\Program Files\Microsoft Office\Office\EXCEL9.OLB" named_guids include("IFont") include ("IPicture")

This will make several files in the build directory, if you add these files to your project you can fiddle with the MSO model in C++ And they use get/set-props all over the place.

Adding any of these files will get you a *huge* pile of objects, so add them one at a time, and make folders in the project workspace.
MSADDNDR.tli
MSO9.tli
VBE6EXT.tli
EXCEL9.tli

* This is a prime candidate to for .ncb corruption; so exit MSVC, delete the project’s .ncb file, and reload the project if the objects don’t show up in the classview when you add the .tli files to your project.



Magmai Kai Holmlor
- Not For Rent

That article was interesting, but I've run into several occasions where the overhead of access operations was drastic. National Instruments has a new product called Measurement Studio, and one of the objects in the suite is a template vector object that performs bounds checking on every access… fine for a debug build or VB app, but not a C++ release. Using a double* dArrayThatDoesntSuck = &NIvector[0]... increased the speed of the code by 16 fold (it was on a time-critical path.)

Bounds checking per-access is a good example of operating at the wrong level granulatity. Bounds checking should be performed in ASSERTs, or you should use .begin() .end() type functions which solve all the problemsm.


I don't think there should be any get/puts either, it means somethings wrong. If it's a property that can be twiddled, it's as good as public. If it requires protection or side-effects, it should be an object with methods that cause something to happen. pMyCommandBarMenu->ChangeIconID(12); instead of pMyCommandBarMenu->IconID = 12; pMyCommandBarMenu->propset(L"IconID", 12) which one is clearer as to what is going on? realize all three (would) do the exact same thing.

OOP guidelines also tell us to name methods with strong verbs, and strong nouns. GetProp is *EXTREMELY* weak - it doesn't get any weaker!

Edited by - Magmai Kai Holmlor on August 11, 2001 1:42:41 AM

Share this post


Link to post
Share on other sites
quote:
Original post by Magmai Kai Holmlor
It depends on what you do with it…

Get/Set tuples are generally bad for two reasons.
1) It doesn''t protect the variable, thus destroying any illusion of encapsulation.

Well, generally they don''t, but they could, if you wanted. You could have it so that these members cap the variable to a given range, either silently, or by throwing an exception if you try to exceed the range.
quote:
Bounds checking per-access is a good example of operating at the wrong level granulatity. Bounds checking should be performed in ASSERTs, or you should use .begin() .end() type functions which solve all the problemsm.

This is a good and interesting point which I''ve not heard expressed so succinctly before. If client code needs random access to a group of variables, it should know the upper and lower bounds and should police these itself. And if it only needs sequential access, it can be given the begin and end iterators/positions as you stated. Per-access bounds checking should not exist in Release builds.

quote:
I don''t think there should be any get/puts either, it means somethings wrong. If it''s a property that can be twiddled, it''s as good as public.

I think this is perhaps too strong a stance to take. Using one of the two is quite common, for example. You might want a variable that is read-only: ie. the object maintains it, perhaps changing it, perhaps not, but client code needs to read it. The safe way is to use a Get function to read it from the owning object. Making it public means something else could alter it, which you don''t want. Another example is being able to set an object with a different Strategy object to change its behaviour. You might want a Set function for this, and maybe a Get function to retrieve it. If you encapsulate Strategy creation in the object, that increases class dependencies which is likely to be a bad thing.

I do agree that, whenever you feel the need to use a Get or Set, and especially a pair of them, that is a big warning sign that you''ve not really thought through what you''re trying to achieve. This doesn''t always mean they''re wrong: just that they often are

Share this post


Link to post
Share on other sites
Yup, I''m not entirely confident about why you would want to use get/set in a core component. But they can be useful if you know where to use them...

They are most useful for objects that actively process encapsulated data, methinks. Such as Documents, for example. This makes them good for scripting, and is why VB implements properties this way.

Share this post


Link to post
Share on other sites
Accessors, when inlined, don't take any performance hit, they allow you to do range checking (in debug builds), they allow you to do and validation (in debug builds), they allow you to perform syncronization, they allow you to change the implementation when deriving new classes, they allow you to notify dependents of changes (observer pattern), etc, etc, etc.

Just the range checking and validation alone makes it worth it to use accessors, in my opinion.

As for Magmai's two reasons accessors are bad:

quote:

It doesn't protect the variable, thus destroying any illusion of encapsulation.



I totally disagree for the same reasons Kylotan specified: automatic range checking and validation (in debug builds). However, if you mean that the user can still twiddle with the memory directly to change the variables, that is true, but that's hardly a downside. The reason (or one of them) for encapsulation is to make it harder for the programmer to make accidental mistakes. If the programmer specifically wants to directly change the value of a memory address, then by all means, go for it.

The other reason Magmai stated is:

quote:

Get/Set methods that perform task can create side-effects that are highly undesirable. If twiddling a control from code sinks an event, you can end up with ugly loops and crappy boolean state variables. Imagine if UpdateData(FALSE) caused each UI object to execute it's OnChange event... this is the sort of thing that happens in VB and is caused by the set/get side-effects



This is a general design issue and has nothing to do with accessors themselves.

The only valid reason posted for not using accessors is that they 'are that they are a pain in the arse'. I can only assume the poster meant it take so much longer to type Set or Get and function parenthesis. Granted, that does take longer to type, but so does proper description function/variable names and comments, but it's still a good idea to do so.

And for those very few classes that wouldn't even use range checking and validation, I'll still use accessors. Why? Because there is no speed decrease and doing so will allow me in the future to make changes easily to that class or deriviations. You should always strive for reusability.


Update: BTW, I totally forgot to cover the consistancy issue. How confusing will it be to programmers when they must use accessors to change variables in class A (because syncronization must exist), but can only change variables directly in class B? Worse yet, what about class C that requires some variables to be changed directly, and others to be changed using accessors? How will the programmer ever know which form to use?

Consistancy is a requirement when other people will be using your code.


- Houdini

Edited by - Houdini on August 27, 2001 12:22:53 PM

Share this post


Link to post
Share on other sites
I guess you can validate the data request using a get/set, but I still think there''s something bad afoot when these guys crop up.

quote:

quote:
--------------------------------------------------------------------------------
Get/Set methods that perform task can create side-effects that are highly undesirable. If twiddling a control from code sinks an event, you can end up with ugly loops and crappy boolean state variables. Imagine if UpdateData(FALSE) caused each UI object to execute it''s OnChange event... this is the sort of thing that happens in VB and is caused by the set/get side-effects
--------------------------------------------------------------------------------

This is a general design issue and has nothing to do with accessors themselves.


True enough, but it''s a pervasive design flaw with this method - it must be too tempting to make setting the variable automatically take-care of updating the state of the object when you have a function to put code in.


If you need to do bounds checking, there''s more than one object - overload the [] or () operator and do debug bounds checking there, instead of a get/set.

If setting the variable has a side effect, it really ought to be a method with an informative name, so the programmer can control when it happens.

If the data needs protection (like multithread sync.) then use a Lock/UnLock pair that returns & accepts a pointer (like DirectDraw with surfaces). It''s more informative and more efficent.

The only remaining case for a get/set touple is direct access to the underlying data, which destroys encapsulation :p It may as well be public. If this is your case, MSVC has built-in support to automatically construct and call the get/set tuple for you (like VB does), removing the pain-in-the-arse/inconsistency problem.


I''m not against accessors - I''m against the Get/Set naming convention.

Magmai Kai Holmlor
- Not For Rent

Share this post


Link to post
Share on other sites
quote:

True enough, but it''s a pervasive design flaw with this method - it must be too tempting to make setting the variable automatically take-care of updating the state of the object when you have a function to put code in.



True, and perhaps to most newbies this would make perfect sense. However, I still believe this is not a reason to stay away from accessors.

After all, the same logic could be applied to an object oriented program. When a newbie creates a Player class it sounds perfectly logical to include an instance of Model, or even derive Player from Model. However, as they gain experience they realize that having every player contain seperate instances of the same model is a Bad Thing.

Does this mean people should stay away from using object oriented programming? No, it just means they''ve got to be careful, and remember to take everything into consideration when creating a design.

quote:

If you need to do bounds checking, there''s more than one object - overload the [] or () operator and do debug bounds checking there, instead of a get/set.



True, you could do that as long as the class is basically a container of that variable. And in those cases it makes perfect sense to do so.

quote:

If setting the variable has a side effect, it really ought to be a method with an informative name, so the programmer can control when it happens.



It will have an informative name. If I create a platform independant graphics class, and I want to change the render mode to wireframe I would do so like this:

SetRenderMode(RENDER_WIREFRAME);

It''s informative, easy to understand, and give the OpenGL or DirectGraphics versions the chance to use their correct API specific constant that match the generic constant passed.

quote:

The only remaining case for a get/set touple is direct access to the underlying data, which destroys encapsulation :p It may as well be public. If this is your case, MSVC has built-in support to automatically construct and call the get/set tuple for you (like VB does), removing the pain-in-the-arse/inconsistency problem.



I always thought the whole reason for accessors is to NOT allow direct access to the underlying data .

As for MSVC''s automatic construct of the get/set tuple, I''ve never heard of it, so I can''t comment on it. Is it even portable?

quote:

I''m not against accessors - I''m against the Get/Set naming convention



That''s no biggie. Just call them Retrieve/Put


- Houdini

Share this post


Link to post
Share on other sites
Well, let''s begin with the fact that VB programmers are just that - VB programmers. They really don''t care about side effects - in fact, many even welcome them. You''re right about information hiding, yes - "encapsulation" is actually a form of information hiding, and in information hiding you don''t change persistent state variables directly. Information hiding, I believe, encourages defining actions on data instead of data. However, while this is a concern OF the client, it turns out that it''s a concern FOR the component.

Under Visual Basic, the client wants to change the window''s state directly. The command Object.State = Value in essence tells the system to do this:

object->SetState (Value);
right?

But wouldn''t that (I''m talking about Visual Basic here) in fact make it easier to track side effects? If an unhealthy side effect is happening as a result of a VB set, it must be a bug (or, indeed, bad design).

But when building systems from scratch, it''s better to define them through functionality than through data. Leave the data to the scripters (game scripters or the businessfolk who write Word macros).

Share this post


Link to post
Share on other sites
Hmm, glad a use Delphi5.
In object Pascal this whole argument about using getter & setter is mute. Object Pascal has a neat keyword called "property"

This allows the "Property" to be treaded as a variable regardless of if it uses getters & setters or directly accessing the variable. Also you can have read only or write only (who would use these?) variable. You can mix what the reader and writer is, ie don''t have to use a reader method if you use a writer method.


PS
Some Objects Pascals limitations are :
Cant write a OS or device drivers.
It is harder to write ofusious code

I think I can life with these limitations..

Share this post


Link to post
Share on other sites
quote:
Original post by ggs
Hmm, glad a use Delphi5.
In object Pascal this whole argument about using getter & setter is mute.

No, it''s not (and I think you meant moot, not mute?)

There is still the issue of whether it is a ''good'' thing, to have all these members exposed as properties for anyone to tweak. Generally, it''s not, because then your object is less of a ''black box''.

Share this post


Link to post
Share on other sites
Borland c++ has a variation of the object pascal "property" keyword. This, in the public section:

__property int Width = {read=GetWidth, write=SetWidth};

Then:

Obj->Width = 5;

...will call Object->SetWidth(5). Could be good if it became part of the c++ standard.

Share this post


Link to post
Share on other sites
The C# syntax is a bit different:
  
public class Bleh
{
private int bla;

public int Bla
{
get
{
return bla;
}
set
{
bla = value;
}
}
}


Now you can do stuff like:
  
Bleh bleh = new Bleh();
bleh.Bla = 42;

Which will call the set thingie(method/whatever).


Edited by - Arild Fines on September 5, 2001 6:45:31 AM

Share this post


Link to post
Share on other sites