Getting rid of Get/Set functions

Started by
34 comments, last by snk_kid 19 years, 7 months ago
Sounds like you're right ;)

I know it's better to create functions that you can use to acces the API trough your own interface..

But it results in a lot of functions (10 set/get functions is not a lot if you're going to create your own renderer interface) so I'm still thinking about it ;)
Advertisement
The GetDevice() call is wholly redundant. Why not just renderDevice->SomeFunction()?

After all, the Renderer class should know how to *render* things instead of providing a "GetDevice()" accessor which then knows how to render things.
--God has paid us the intolerable compliment of loving us, in the deepest, most tragic, most inexorable sense.- C.S. Lewis
Quote:Original post by CoffeeMug
Quote:Original post by Magmai Kai Holmlor
In all areas of programming, get/set tuples are a code smell.

Well, what about a trivial example like this:


Doesn't matter; set/get tuple are only useful if they do something. If all they do is drop the object back, they're not useful (fail to abstract). If they actually do something, then they are poorly named.
Get/set is poor programming for essentially the same reason the use of the word "get" is poor grammar. You should be more explicit - get/set doesn't tell you anything.

For the first case, I hope it's clear to everyone that the triangle code above is absolutely pointless and makes the class *harder* to use and understand.

Your Get/SetWindowSize example is the second case. I'd call it Window::Resize, you're not just setting the size (cached for later use?), you're causing an avalanche of changes.

GetWindowSize isn't so objectionable, but the 'get' still doesn't tell you anything. Size is OK, but slightly ambiguous (size of the client/viewport area?), I think I'd use Window::Extents, though it wouldn't be hard to convince me to use Window::Size.

It might seem trivial, but code is names. The code still compiles if all the classes and variables are named x1, x2, x3, etc...
- The trade-off between price and quality does not exist in Japan. Rather, the idea that high quality brings on cost reduction is widely accepted.-- Tajima & Matsubara
highly interesting discussion.
just to clarify your point, Magmai Kai Holmlor -
Quote:
original post by Magmai Kai Holmlor
Doesn't matter; set/get tuple are only useful if they do something. If all they do is drop the object back, they're not useful (fail to abstract). If they actually do something, then they are poorly named.

so are you saying that the difference between these functions
// 1.void SetX( int nX ) { m_x = nX;  }// 2.void SetX( int nX ) { if( nX >= 0 && nX < some_predefined_variable ) { m_x = nX; } }// 3.void RedefineX( int nX ) { if( nX >= 0 && nX < some_predefined_variable ) { m_x = nX; } }

is that 1. is not useful because it doesn't do something, but 2. is actually useful because it does ? if so, this begs the question on how much functionality a function should have before it is classified above a petty "setter" or "getter".
edit : and apparently 3. is the best because of its more descriptive name.
- stormrunner
Quote:Original post by antareus
[...]The problem with properties is that you essentially have to store two member function pointers for a little syntactic sugar.[...]
Are you sure that it wouldn't be inlined if coded properly? You could make the function pointers template parameters, in which case it should be done entirely compile time with no need to actually store the function pointers. You *would* have to have a 'parent' pointer of some kind, but maybe the compiler would be smart enough to make all the properties of a class share the same single pointer or somesuch if they stored a reference or something like that?
"Walk not the trodden path, for it has borne it's burden." -John, Flying Monk
Quote:Original post by Scheermesje
class CPoint:{ float getX() const { return x; } float getY() const { return y; } float getZ() const { return z; }   void setX(float X)  { x = X; } void setY(float Y)  { y = Y; } void setZ(float Z)  { z = Z; }private;float x,y,z;}



classic example of complete redundancy

1. In that example the getter/setters does not provide any abstraction what so ever.

3. You may at first believe this provides loose coupling among types/modules, if you think about it really it does not in any way for every member you have a corresponding get"insert member name" and set"insert member name" so clients still are indirectly tightly coupled to representation.

3. A point in 3-space is always representated by a tuple (3 scalar values), there is no variation in representation thats one reason to not make it private. There is just one thing thou a tuple could be representated by named variables or an array of 3 elements but there is good way to "Have Your Cake and Eat It Too" i'll demonstrate at the end.

4. A point in 3-space has no state invariant to maintain, what would it mean to have one besides? so thats another reason to not make it private.

This is just to restrictive for no good reason what so ever.

"Have Your Cake and Eat It Too"


#include <cstddef>template < typename T >class point3 {   typedef T point3<T>::* const p3[3];	//<--- pointer to data members   static const p3 v;          //<- private, clients never know about it.public:    typedef size_t size_type;    T x, y, z;    /* ... omitting code ... */    const T& operator[](size_type i) const {       return this->*v;    }    T& operator[](size_type i) {       return this->*v;    }    /* ... omitting code ... */};template < typename T >const typename point3<T>::p3 point3<T>::v = { &point3<T>::x, &point3<T>::y, &point3<T>::z };


Before anyone trys to post anything about using anonymous unions read this thread completely first.

This topic is closed to new replies.

Advertisement