Style question: SetShutdown/GetShutdown .vs. SetShutdown() overridden

J

Jim Langston

It is common, and generally a good idea, to have setters and getters for
class variables. I.E.

class MyClass
{
public:
void SetShutdown( const bool sd ) { Shutdown = sd; };
bool GetShutdown() { return Shutdown };
private:
bool Shutdown;
};

What I'm contemplating is instead of set and get simply overriding:

class MyClass
{
public:
void shutdown( bool sd ) { Shutdown = sd; };
bool shutdown() { return Shutdown; };
private:
bool Shutdown;
};

Now, this is strictly a style question, and it gets a little confusing with
the only difference between the function and the variable the capital 'S'.
But do you, personally, think this style is a good idea and do you like it?

Or can you think of some other way instead of SetXxxxx() and GetXxxxxx() ?
 
I

Ian Collins

Jim said:
It is common, and generally a good idea, to have setters and getters for
class variables. I.E.
Some, me included, consider setters and getters to be a design smell,
ask yourself why these members requite access from outside, do they
belong in this class, or the one that access them?
 
J

Jim Langston

Ian Collins said:
Some, me included, consider setters and getters to be a design smell, ask
yourself why these members requite access from outside, do they belong in
this class, or the one that access them?

Well, take this specific example that I'm using Shutdown for. It's for a
threaded queue class to communicate between the worker threads (socket
connections) and the main thread (server program). There is a method to
push strings onto the queue (which the worker threads use), a method to pop
messages from the queue (which the main thread uses) with varius threading
code to ensure this class is thread safe. There is even a method that will
send a shutdown method onto the queue, that when poped will exit the queue,
rather ungracefully I might add.

Adding the variable shutdown is simply so the main thread can set this
variable (no locking needed) so the worker threads can check it periodically
to shut down gracefully.

That, in my opionion, means that this variable does belong in the class and
there needs to be a method to set it, and a method to check it. Yes, rather
than a simple SetXxxx and GetXxxx I could use more syntactical method names
( SendShutdown() and IsShutdown() or something ) but Set and Get are common.

Now, for other instances of SetXxxx and GetXxxx I have used, in one graphic
class I use:

void CGUIElement::SetLocation( unsigned long X, unsigned long Y )
{
BitmapX = X;
BitmapY = Y;
};

Which simply moves the bitmap to a new location. This could be used for
moving windows around, dragging icons, whatever. One could argue that the
bitmap locations could be stored in the class that sets them, but why? They
are part of the GUI object itself.

Another example from this same GUI class is:

void CGUIElement::SetOpen( bool OpenIt )
{
if ( OpenIt && !IsOpen )
{
CloseOnOpenNotifier.notifyObservers( dynamic_cast<Argument*>(
&OpenArgument( NotifyMsg ) ) );
IsOpen = true;
OpenNotifier.notifyObservers( dynamic_cast<Argument*>( &OpenArgument(
NotifyMsg ) ) );
}
else if ( !OpenIt && IsOpen )
{
IsOpen = false;
CloseNotifier.notifyObservers( dynamic_cast<Argument*>( &OpenArgument(
NotifyMsg ) ) );
}
};

and as you can see this is more than a simple assignment of a variable.
This "setter" actually does quite a bit, but is still a setter, and is
needed not only to let the GUI itself know that it's "open" (a window for
instance) but utilizing the observable model inform the watchers that it
opened or closed.

There are many other examples I can give you, and I don't think that Setters
and Getters are neccessarily a design smell just because they exist. They
may be given more appropo names perhaps (SetOpen could be broken up into
Open and Close for instance) but the fact remains they are still Setters and
Getters.
 
A

Alf P. Steinbach

* Jim Langston:
Adding the variable shutdown is simply so the main thread can set this
variable (no locking needed) so the worker threads can check it periodically
to shut down gracefully.

In which cases would you ever call

thread.SetShutdown( false );

?

As I see it, either it will be redundant because you know shutdown has
not been set to true earlier, or, the call will be wrapped in some
synchronization that may fail, or, the call may silently have no effect
because the thread has already terminated (and then be of no use unless
you're really keen on reusing a possibly non-terminated thread).

At a sufficient high level of abstraction the same is very often the
case: exposing a value makes both client code and class implementation
much more complex and unsafe than necessary, by removing constraints
that are useful both to the client side and the implementation.
 
I

Ian Collins

Jim Langston wrote:

[good points snipped]
There are many other examples I can give you, and I don't think that Setters
and Getters are neccessarily a design smell just because they exist. They
may be given more appropo names perhaps (SetOpen could be broken up into
Open and Close for instance) but the fact remains they are still Setters and
Getters.
I guess I was being more general, I've seen a lot of code written by OO
novices that is riddles with set/get methods due to poor design. Your
examples where some of the exceptions that proves the rule.

My advice still stands, do think hard about your design if private
members are being manipulated from outside.
 
J

Jim Langston

Alf P. Steinbach said:
* Jim Langston:

In which cases would you ever call

thread.SetShutdown( false );

?

As I see it, either it will be redundant because you know shutdown has
not been set to true earlier, or, the call will be wrapped in some
synchronization that may fail, or, the call may silently have no effect
because the thread has already terminated (and then be of no use unless
you're really keen on reusing a possibly non-terminated thread).

At a sufficient high level of abstraction the same is very often the
case: exposing a value makes both client code and class implementation
much more complex and unsafe than necessary, by removing constraints
that are useful both to the client side and the implementation.

Well, then, what about my example for void CGUIElement::SetOpen( bool
OpenIt )
I do call it with both true and false. true so a window is visible, false
when it's to disappear. I also have a method to see if a window is open or
not. Although in this case it seems I didn't use IsOpen for some reason
but:
bool Open() { return IsOpen; };
void SetOpen( bool OpenIt );

I guess having a variable IsOpen confused me. I'm really not happy with the
variable names or method names for this class, which is another reason I'm
trying to find better.

One alternative is to have method names:

bool IsOpen();
void SetOpen();
void SetClosed();

or OpenIt, CloseIt or something. I don't know. That's why I'm trying to
see what other people use.

I really don't like splitting up SetOpen() into two methods, however, as the
observable/observed model gets tricky for me and I want to see what I'm
doing in one function for opening and closing.
 
H

Heinz Ozwirk

Jim Langston said:
Well, then, what about my example for void CGUIElement::SetOpen( bool
OpenIt )
I do call it with both true and false. true so a window is visible, false
when it's to disappear. I also have a method to see if a window is open
or not. Although in this case it seems I didn't use IsOpen for some
reason but:
bool Open() { return IsOpen; };
void SetOpen( bool OpenIt );

I guess having a variable IsOpen confused me. I'm really not happy with
the variable names or method names for this class, which is another reason
I'm trying to find better.

All the examples you have shown in this and previous posts are actually
actions you want the object to perform.

You want a queue to be cloese (or shutdown graciously), so say so --
queue.Close() -- and if you want to know if it is closing, ask it -- if
(queue.IsClosing()).

The same hold for the bitmap's location. You don't want to set its location.
You want to move it somewhere. So say so -- guiElement.MoveTo(x, y). When
you want to know its position, ask it -- guiElement.GetPosition(x, y).

And how to show or hide something on the screen? Say so -- guiElement.Show()
or guiElement.Hide() -- and ask it to get its state -- if
(guiElement.IsVisible())

I admit that there is no symetry like GetSomething and SetSomething, but all
the setters you mentioned actually had to do more than simply modify a
variable, and even if they did, that is something the called does not need
to know.

The client of a queue does not care how closing a queue is implemented. He
wants the queue to be closed, shut down or whatever you call that. Moving a
bitmap might require more action than simply changing some variables. Most
likely you want the bitmap actualy be shown at its new location on the
screen. And i you tell a GUI element to show or hide itself, you want it to
be shown or hidden on the screen. You don't want a GUI element that is
virtually visible or hidden.

Sometimes there are classes with properties that can be manipulated directly
by the client, but whenever you are writing setters, you should ask yourself
"What do I really want that object to do?"

And to your original question -- use whatever naming scheme you like, but
use it consistently. And make the getters const.

HTH
Heinz
 
B

Ben Pope

Ian said:
Jim Langston wrote:

[good points snipped]
There are many other examples I can give you, and I don't think that
Setters and Getters are neccessarily a design smell just because they
exist. They may be given more appropo names perhaps (SetOpen could be
broken up into Open and Close for instance) but the fact remains they
are still Setters and Getters.
I guess I was being more general, I've seen a lot of code written by OO
novices that is riddles with set/get methods due to poor design. Your
examples where some of the exceptions that proves the rule.

My advice still stands, do think hard about your design if private
members are being manipulated from outside.

When I was being taught Java, I think there was too much emphasis on OO,
everything had to be an object. An object was always considered as the
fundamental building block, and as much as possible related to physical
entities. IMO, the concept of an object was wrong.

All of the programs I saw were like:

something.GetThis().GetThat().DoThis();

Eurgh.

But if you want to write programs like that, Get/Set are fundamental.
Of course, this completely throws away encapsulation, increases coupling
and makes refactoring nigh on impossible.

Going a little off topic now, does anybody else think that Get/Set is a
Java-ism? I haven't coded any Java for a long time, but I've just taken
quick gander at the API reference and I can't find a single class that
doesn't have at least one get and set.

Ben Pope
 
M

Marcelo Pinto

Jim Langston escreveu:
It is common, and generally a good idea, to have setters and getters for
class variables. I.E.

IMHO I don't believe it's a good idea in general. You may just be
giving up encapsulation. I have seen classes with default constructors
and a bunch of setters to put the object in a consistent state, this is
simply violating the information hiding principle, and an unnecessary
risk of letting the object in an invalid state. Generally I like to
keep the internals of my class really internal. If there is a
justifiable reason I provide a "Getter" (never with the Get prefix) and
if there is a *very* justifiable reason I provide a "Setter" (never
with the Set prefix) to alter one aspect of my object's state.
class MyClass
{
public:
void SetShutdown( const bool sd ) { Shutdown = sd; };
bool GetShutdown() { return Shutdown };
private:
bool Shutdown;
};

What I'm contemplating is instead of set and get simply overriding:

class MyClass
{
public:
void shutdown( bool sd ) { Shutdown = sd; };
bool shutdown() { return Shutdown; };
private:
bool Shutdown;
};

Now, this is strictly a style question, and it gets a little confusing with
the only difference between the function and the variable the capital 'S'.
But do you, personally, think this style is a good idea and do you like it?

I personally use the teminating underscore (as Sutter & Alexandrescu
suggest):
bool shutdown_;
Or can you think of some other way instead of SetXxxxx() and GetXxxxxx() ?

You may provide constructors tha put your object in the correct state
(as explained above). You may, if the application logic requires,
provide functions that alter the state of the object without messing
with individual parts of the object...

Just my $0.02,

Marcelo Pinto
 
T

TB

Marcelo Pinto sade:
Jim Langston escreveu:


IMHO I don't believe it's a good idea in general. You may just be
giving up encapsulation. I have seen classes with default constructors
and a bunch of setters to put the object in a consistent state, this is
simply violating the information hiding principle, and an unnecessary

The absolute enforcement of that principle is debatable. The actual
meaning of the concept still varies, even among languages that try
to structure themselves around it.
 
M

Marcelo Pinto

TB escreveu:
Marcelo Pinto sade:

The absolute enforcement of that principle is debatable. The actual
meaning of the concept still varies, even among languages that try
to structure themselves around it.

I didn't understand what you meant. Could you please clarify?

TIA,

Marcelo Pinto
 
D

Daniel T.

"Jim Langston said:
It is common, and generally a good idea, to have setters and getters for
class variables. I.E.

More specifically, it is a bad idea to make member-variables public.
"getters" do no harm as long as they don't expose any internals but they
may confuse the programmer using your class, it may be information he
never really needs to know about so he is wondering why the getter is
there. "setters" are fine *if and only if* calling the setX
member-function cannot possibly put the object in an inconsistent state,
and again it may be information that the user never actually needs to
set...

The important point is this: getX/setX do not, in and of themselves,
break encapsulation... unless you change them every time you change the
internal representation of the class.
class MyClass
{
public:
void SetShutdown( const bool sd ) { Shutdown = sd; };
bool GetShutdown() { return Shutdown };
private:
bool Shutdown;
};

What I'm contemplating is instead of set and get simply overriding:

class MyClass
{
public:
void shutdown( bool sd ) { Shutdown = sd; };
bool shutdown() { return Shutdown; };
private:
bool Shutdown;
};

Now, this is strictly a style question, and it gets a little confusing with
the only difference between the function and the variable the capital 'S'.
But do you, personally, think this style is a good idea and do you like it?

Or can you think of some other way instead of SetXxxxx() and GetXxxxxx() ?

In my own code, I tend write it so that if the client code is read out
loud, it sound right.

if ( myObject.isShutdown() ) // "If my object is shutdown"

sounds better to me than:

if ( object.showdown() ) // sounds more like an imperative

The best way to decide what a function should be named IMHO is to look
at how it is used in client code, not by staring at the internal
representation of the class.
 
B

Bo Persson

Ben Pope said:
Going a little off topic now, does anybody else think that Get/Set
is a Java-ism? I haven't coded any Java for a long time, but I've
just taken quick gander at the API reference and I can't find a
single class that doesn't have at least one get and set.

I think it is OO fundamentalism. You are tought to make all object
states private. Having a public member of an object is just sinful!

Right!

So you end up with code exactly like the example at the start of the
thread:

class MyClass
{
public:
void SetShutdown( const bool sd ) { Shutdown = sd; };
bool GetShutdown() { return Shutdown };
private:
bool Shutdown;
};


Except for that it follows the OO Comandments to the letter, it also
get around the rule without breaking them (sort of).

But what exactly is the advantage over

struct MyClass
{
bool Shutdown;
};

or just

bool Shutdown;

??

Even with the private variable, *everyone* can read it, and change it
to whatever value they want! There is no real protection, just a lot
of extra work, but no benefits.



Excuses to the original poster. No offence intended. :)


Bo Persson
 
D

Daniel T.

"Bo Persson" <[email protected]> said:
I think it is OO fundamentalism. You are tought to make all object
states private. Having a public member of an object is just sinful!

Right!

So you end up with code exactly like the example at the start of the
thread:

class MyClass
{
public:
void SetShutdown( const bool sd ) { Shutdown = sd; };
bool GetShutdown() { return Shutdown };
private:
bool Shutdown;
};


Except for that it follows the OO Comandments to the letter, it also
get around the rule without breaking them (sort of).

But what exactly is the advantage over

struct MyClass
{
bool Shutdown;
};

or just

bool Shutdown;

??

Even with the private variable, *everyone* can read it, and change it
to whatever value they want! There is no real protection, just a lot
of extra work, but no benefits.

That would be true, if we mandated that forever and always, GetX and
SetX *must* get/set a particular member-variable. That fortunately isn't
the case.

class MyClass {
// private implementation
public:
void setShutdown(bool flag);
bool getShutdown() const;
};

Now, must it always be the case that setShutdown sets a member-variable,
and getShutdown returns the value of that variable? No. For all you know
(without looking at the private parts of the class) setShutdown may
actually create an IP packet to be sent to another computer...
 
I

Ian Collins

Bo said:
I think it is OO fundamentalism. You are tought to make all object
states private. Having a public member of an object is just sinful!

Right!

So you end up with code exactly like the example at the start of the
thread:

class MyClass
{
public:
void SetShutdown( const bool sd ) { Shutdown = sd; };
bool GetShutdown() { return Shutdown };
private:
bool Shutdown;
};


Except for that it follows the OO Comandments to the letter, it also
get around the rule without breaking them (sort of).

But what exactly is the advantage over

struct MyClass
{
bool Shutdown;
};

or just

bool Shutdown;
Nothing, assuming the object does noting else.
 
J

Jim Langston

Jim Langston said:
It is common, and generally a good idea, to have setters and getters for
class variables. I.E.

class MyClass
{
public:
void SetShutdown( const bool sd ) { Shutdown = sd; };
bool GetShutdown() { return Shutdown };
private:
bool Shutdown;
};

What I'm contemplating is instead of set and get simply overriding:

class MyClass
{
public:
void shutdown( bool sd ) { Shutdown = sd; };
bool shutdown() { return Shutdown; };
private:
bool Shutdown;
};

Now, this is strictly a style question, and it gets a little confusing
with the only difference between the function and the variable the capital
'S'. But do you, personally, think this style is a good idea and do you
like it?

Or can you think of some other way instead of SetXxxxx() and GetXxxxxx() ?

Ian, Alf, Heinz, Ben, Bo, Daniel, Marcelo, TB, et al, all very good points
which made me re-evaluate setters and getters.

The user of my class (usually me) isn't setting or getting variables. They
are attempting to set some state of the object or get information about it.
The user doesn't care what I call my variables in my class, they just want
to execute methods on the class.

So, SetXxxx and GetXxxx is out. If my class needs the Shutdown variable set
to perform some action, make the method name state that.
void RequestShutdown();
bool IsShutdown() const;

The fact that I'm setting a variable called Shutdown to true when they call
this method should be transparent to them.

In my GUI where the user is updating the GUI position, that is what they're
doing, they're not setting my X and Y variables.
void UpdatePosition( const int x, const int y );

In my GUI where the user is updating requesting the GUI to open or close,
that is what they're doing, they not updating my Open varaible, that should
be transparent to them.

void Close();
void Open();
bool IsOpen();

I use open and close instead of visible, because the GUI can be a window and
they actually close including all their sub GUIs

I think this change will be beneficial to my classes in lots of ways. Any
time I find myself wanting to type SetXxxx or GetXxxx I'l think about what
the user of my class is really doing.

Thanks a lot for your input.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,906
Latest member
SkinfixSkintag

Latest Threads

Top