The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

accessor functions

I'm writing a new class and I want to provide the ability to request certain information from it.  I've flip-flopped on the best way to do this.  I can have individually named methods ie.:

int GetHeight();
int GetWidth();
etc...

or I can create on method and specify what i want in an enumerated param ie.:

enum {
HEIGHT,
WIDTH
}

h = Get(HEIGHT);
w = Get(WIDTH);

any opinions?
flip-flopper
Monday, November 22, 2004
 
 
The first way.  If you want other people to use your code, or languages other than what you have written in, that's pretty much how they will expect to see it.
Aaron F Stanton Send private email
Monday, November 22, 2004
 
 
I'd say the first way too.

Alternative:

Size  GetSize();
Jonathan Send private email
Monday, November 22, 2004
 
 
The first, for quite a few reasons.

- easier to debug,
- easier to override one particular accessor,
- much more standard,
- flexibility (what if HEIGHT is a float, and AGE is an int?)
- possible namespace collisions,
- code readability,

Of course there are always exceptions, but given the most simple context, the former comes out on top.
Nigel Send private email
Monday, November 22, 2004
 
 
The first one, anyday in any language. The second one wont scale if you require more accessories like the previous poster stated. Also, you will almost always want to add a const to all your accessors. int getHeight(void) const { ... }
girish ramakrishnan Send private email
Monday, November 22, 2004
 
 
Neither one.

Classes shouldn't provide information, they should do things. That's what Holub taught me.

  Flava Flav!
Flava Flav
Tuesday, November 23, 2004
 
 
Why are writing accessors in the first place? Give out useful methods that 'do' things, as opposed to methods that just spill out the internals and force the client to write the useful code.
subhash Send private email
Tuesday, November 23, 2004
 
 
So, if I want to find out the size of my window, I do what exactly?

Modify the vendor-supplied "Window" class to persist the size in the registry with some kind of SaveSizeToRegistry() function?

And since when has class extension outside of the class declaration been possible in mainstream languages, say, C++? (Assuming I don't want to modify the vendor-supplied library source code).

By your logic, you'll be writing off much of the .NET library, I guess.
Jonathan Send private email
Tuesday, November 23, 2004
 
 
Find out the size of the window and do what .. increase it? decrease it?

Or find the clipped space with another window?

These are useful methods in the Window class, and someone who designs the Window class should think of these possible methods as candidates for their class. If they are coding against (sic) a set of library APIs, they don't have a choice of course.
subhash Send private email
Tuesday, November 23, 2004
 
 
See, out here in the real world, software often has to provide information about its state. You know, so you can display that information to the user. Apply your logic to a car, and you have accelerator and break pedals, but no way to see how fast you're going.

Having your middle tier code directly interact with your user interface is A Bad Thing (ever hear of partitioning? Modularity? Re-use?), so using a method for this task it out. The answer is to communicate state information using a property (or if you're using a property-deficient language, an accessor).

And to answer the OP question, use GetWhatever(), without question. The accessor paradigm is bad enough to begin with without adding enums into the equation. Yuk.
.NET Guy
Tuesday, November 23, 2004
 
 
No, they don't have a choice. But neither can the designer of the Window class cater for every possible use. Maybe I need to find the positions of the windows, so I can arrange them in a slightly non-standard way for a special multimedia app, perhaps, that the class designer could not possibly envisage.

Which is why GetSize / SetSize accessors (or properties) are commonly used as a compromise.

My opinion was given because I was driving at the following idea: A call along the lines of:

DoSomething( MyWindow.GetSize() )

is better than:

DoSomething( pMyWindow->m_Width, pMyWindow->m_Height )

... because it DOES hide some internals: Maybe some windows store their size as two member variables, but others compute their size based on other factors.

I understand where you're coming from, but I still think you're reading too much into this, given the original poster's examples.
Jonathan Send private email
Tuesday, November 23, 2004
 
 
Yup, definitely the ..GetHeight() construct.  Using the 'Get(HeightVal)' construct leads to poorly coupled code, as the comments above indicate.  Poorly coupled code becomes a nightmare to maintain.  And I agree that the 'MyWindow.GetHeight()' construct is much more self documenting (and safer to change in the future) than the string of arrows you can wind up with.
AllanL5
Tuesday, November 23, 2004
 
 
Reading Win32gui I found this nice approach:

p->text("New Title");  // setter
string s = p->text();  // getter

Obviously achievable with overloading. The advantage is that you avoid the verbiage of Get/Set and there is "one" function per property.
Alex Send private email
Wednesday, November 24, 2004
 
 
I use accessors very often, but I make most of them private.
Pakter
Wednesday, November 24, 2004
 
 
Alex,

That's smalltalk accent ..

In Smalltalk, there is no way to declare the data members public. If you want to give clients access to the data members of a class, the only way to do that is to create accessors. Another nudge towards thorough encapuslation ..

Back to the naming part, it really looks cleaner than get/set as in :

r = circle radius;
circle radius: r+5;
subhash Send private email
Thursday, November 25, 2004
 
 
The conventional wisdom is that accessors are bad.  Classes should do things not provide information...etc, etc.  I've heard this idea from none other than Bjarne Stroustrup.

I just don't buy it.

No accessors means that if I have a coordinate class P, with a constructor P::P(int x, int y), there should be no way to get x or y out of the coordinate.  I asked this very question of some expert promoting this "no accessors" idea at a conference talk and his answer was "let's take this discussion off-line".

No accessors is really a theoretical programmer versus working, boots-on-the-ground programmer debate.

I like the first method.  More elegant in my view.  Elegance is important (Also a Stroustrup idea).

However the second method (assuming a real application with 1000's of accessors) might reduce public names which will in turn reduce DLL or assembly loading time and so I might refactor to the second method during performance tuning.
Bob Rundle Send private email
Friday, November 26, 2004
 
 
For classes which provide information, accessors are, of course, the way to go. The trouble is that for many classes accessors are provided where they're not needed and more active functions would provide better encapsulation and object orientation.

In the classic pseudo-example, a car object should provide Accelerate() and Brake() not SetSpeed().
Mr Jack
Friday, November 26, 2004
 
 
"In the classic pseudo-example, a car object should provide Accelerate() and Brake() not SetSpeed()."

Actually SetSpeed() would, in some cases, be much more useful than Accelerate() and Brake().  Cruise control works on this principle -- To SetSpeed() and then let the car Accelerate() or Brake() as needed to maintain that speed.

Also, while you can get away without SetSpeed() you really need the GetSpeed() accessor is no matter what.

I guess another pseudo-example is required.
Almost Anonymous Send private email
Friday, November 26, 2004
 
 
I think this has gone off-topic (my fault too!). The original poster didn't ask whether accessor functions are good or bad.

There is also one more issue I would highlight, although I think a couple of others hinted at this.

The first example suggests that the function is called directly. The second example suggests that an extra dispatch is needed at run-time, something along the lines of:

switch( message )
{
  case HEIGHT: return m_Height;
  case WIDTH:  return m_Width;
}

Which may well be more time-expensive.

Unless the compiler inlines the above into the call sites, throwing away the other switch cases since only one 'case' will ever apply when the 'message' parameter is a constant.
Jonathan Send private email
Friday, November 26, 2004
 
 
That's a good point.  I inline all my all small accessor functions -- so really my accessor functions are just syntactic sugar.
Almost Anonymous Send private email
Friday, November 26, 2004
 
 
Philosophic arguments in response to specific implementation questions make me want to go out and break something.
Bored Bystander Send private email
Saturday, November 27, 2004
 
 
Like I said, Get(HEIGHT) and Set(HEIGHT) would imply a #define/enum for HEIGHT somewhere (probably global scope), which is messy. Global scope has no business knowing that some class somewhere has a property called "height".

And if you're a brave student with high grades, you might place them inside a class/namespace, leading to amazing constructs like Get(Size::HEIGHT) and Set(Size::HEIGHT). Ok, hold the applause, thank you.

As you'd expect, the last paragraph brings the voice of reason: who not use Height()? That's the getter, by the way. The setter would be: Height(42); It's called function overloading, one of the "bloated" features in C++.
Alex Send private email
Saturday, November 27, 2004
 
 
You know Almost, I thought about adding that exact comment - but I figured we're all smart enough to understand a pseudo-example when we see one.
Mr Jack
Tuesday, November 30, 2004
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz