Jump to content
  • Advertisement
Sign in to follow this  
Toolmaker

Getting rid of Get/Set functions

This topic is 5448 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Quote:
Original post by Oxyd
I'm getting confused about this [smile] Everyone's talking about "redesigning" the class not to use getter and setters...


No we are not saying that, well i'm not if you read the thread carefully this is what sparked it off:

Quote:
Original post by Toolmaker
...
if you need alot of accessor functions that don't do any checking...


There is no point, if your type's representation does not vary and your type's representation does not have a state invariant to maintain then there isn't any point in making its representation private. Read my last post for more on that.

Also if there really is a need to encapsulate say to maintain the state invariant but you have lots of fine grained getters & setters then you've probably modelled your type at the wrong level or mixed levels of abstraction, its to low-level it should either be redesigned or its should be used in conjunction with another type that is at a higher level for clients to use.

If i saw some ones class with lots of getters & setters i would say that person never designed that class at all they just coded it along the way.

Quote:
Original post by Talonius
So the question begs to be asked: why were they introduced in the first place?


simple they where not, getters & setters are not a concept.

Quote:
Original post by Talonius
Is this a case of the class designers deciding that "hey, this is a great idea, and it builds on the data hiding aspect!"


To hide data is not the same as the reason why to hide it in the first place and its usually to encapsulate variants with an invariant interface to clients that usually maintains state invariants aswell.

Quote:
Original post by Talonius
(Why make a variable private? So no one can mess with it.


Not when you have a setter that does no checking, everyone can still mess with it.

Quote:
Original post by Talonius
Why make a variable protected? So your child classes can access and change it.


Making a data members protected is the biggest contridication ever, this is usually a sign of bad design, "why?" you ask

well your hiding it from un-related types so "no one can mess with it" or in my terms to maintain a state invariant and/or to encapsulate variant representation but making it protected you let everyone mess with it by deriving from it.

Quote:
Original post by Talonius
Why make a variable public? Uh... I don't know, why?)


because your asserting to clients that the type's representation does not or will not change, and there is no state invariant to maintain, and your asserting its logically part of the type's interface because that is how your design turn out to be.

At the end of the day it all depends on the requirements spec of your problem domain.

Share this post


Link to post
Share on other sites
Advertisement
Quote:
Original post by CoffeeMug
What are these extensions for MSVC

__declspec(property(get=GetX, put=SetX)) int X;

If you just want an accessor, leave off the put part.

Quote:
It's suprising that boost doesn't include a property implementation. Perhaps we can design one together and submit it for peer review?

The problem with properties is that you essentially have to store two member function pointers for a little syntactic sugar. I may be Texan at heart but I don't like my objects *that* big. :) Can you imagine how large a WinForm-ish class would be like that?

The property extension on MSVC merely rewrites property gets/sets in-terms of the member functions, even when you say o.X += 1 (expands out into o.SetX(o.GetX() + 1)).

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
Quote:
Original post by Toolmaker
A friend of mine asked me today if I knew a little trick to help him save the hassle of writing 10 get and 10 set methods. So I came up with a little nifty macro magic that might save you from writing those functions too.

If you have basic Getter/Setters that don't go any check, using these macro's might save you 30 minutes of writing boring code.

*** Source Snippet Removed ***

Basic usage is like this:

class Foo
{
public:
GETTERSETTER(float, fScaling);
GETTERSETTER(float, fRotation);
GETTER(std::string, TypeOf);
SETTER(float, WeaponDamage);

private:
float m_fScaling;
float m_fRotation;
std::string m_TypeOf;
int m_WeaponDamage;
};

Foo MyObject;
MyObject.SET(fScaling, 1.0f);
MyObject.SET(WeaponDamage, 10);
std::string Type = MyObject.GET(TypeOf);


Just wanted to share. The macro's aren't THE best solution, but are a quick work around if you need alot of accessor functions that don't do any checking.

Feel free to use this code in whatever way, except to take over the world.

Toolmaker


If any person I was working with were to show me code like this, I would feel a severe urge to strangle them.

Your code is clever. Clever code is bad code, as any experienced software person will tell you. It is useless beyond the "hey, look what I can do" examples like the one you have above.

Share this post


Link to post
Share on other sites
Quote:
Original post by antareus
__declspec(property(get=GetX, put=SetX)) int X;

Thanks. I'd never use this monstrosity though [smile]
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.

I think writing member functions and then wrapping them into a property with two function pointers is idiotic. Just use the damned member functions! A better idea would be some sort of a template solution (perhaps using boost::lambda?) that lets you write accessor code in place and generates a structure for each property that has no memory overhead. Perhaps something that would be used like this:

class MyClass
{
public:
property<int>
(
// accessor
return m_Value;
,
// mutator
m_Value = _1;
) Length;
};

And then

MyClass c;
c.Length = 15;
cout << c.Length;

Share this post


Link to post
Share on other sites
To be annoying: What if we wanted the Triangle class to be simple. Say, you just need it to draw a triangle on the screen (writing your own renderer, or whatever). So it's basically a structure with draw () method... Would it be good idea, to make the vertices public? What if I later wanted to add some fancy feature, like checking wheter the triangle isn't out of screen with the new value?

Oxyd

Share this post


Link to post
Share on other sites
Still a bad example. In practice you'll never call Triangle::Draw() because it's too damn slow to do a function call for each triangle. You'll most likely call DrawTriangles(const vector<Triangle> &Triangles). If you want to ensure triangles don't get out of the screen you should ensure this at a higher abstraction level.

Share this post


Link to post
Share on other sites
Really cool discussion :)

I've seen classes like the following:


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;
}



I've never understood why people did it but I thought it was 'the rule to follow' so I must admit my code looks like this.. After reading this thread I'm really going to remove this stuff :)

But what do you guys think of something like:
Renderer->getDevice()->SomeFunction ();

There is no set function for my device so it shouldn't be public but code with lots of those render->getdevice statements is looking terrible. Is there a solution for this?

Share this post


Link to post
Share on other sites
Quote:
Original post by Scheermesje
But what do you guys think of something like:
Renderer->getDevice()->SomeFunction ();

There is no set function for my device so it shouldn't be public but code with lots of those render->getdevice statements is looking terrible. Is there a solution for this?

I use code like that too. Its a lot better than exposing the m_pDevice (or whatever you call it) member variable, since using a GetDevice() stops someone from setting the pointer to something else and breaking the code. Effectively, its read only.
I don't think it looks ugly at all, but thats just my opinion

Share this post


Link to post
Share on other sites
There was a discussion on a gamedev mailing list..

The discussion went about the following type of statement:

CFramework::Get().Log->Get().Write('');

It are not really set/get functions since it's a singleton but the problem is somewhat similiar.. Should you make the Log file a public member of your Framework or maybe make it global so you can remove all the set/get thingys.

And also if you would change something like this:


Render.getDevice()->SetTextureStageState(0, D3DTSS_ALPHAARG1, D3DTA_DIFFUSE);
Render.getDevice()->SetTextureStageState(0, D3DTSS_ALPHAOP,D3DTOP_SELECTARG1);

Render.getDevice()->SetRenderState(D3DRS_SRCBLEND, D3DBLEND_SRCALPHA);
Render.getDevice()->SetRenderState(D3DRS_DESTBLEND, D3DBLEND_INVSRCALPHA);

to

LPDIRECT3DDEVICE9 TempRender = Render.getDevice();
// Set the alpha stages
TempRender->SetTextureStageState(0, D3DTSS_ALPHAARG1, D3DTA_DIFFUSE);
TempRender->SetTextureStageState(0, D3DTSS_ALPHAOP,D3DTOP_SELECTARG1);

TempRender->SetRenderState(D3DRS_SRCBLEND, D3DBLEND_SRCALPHA);
TempRender->SetRenderState(D3DRS_DESTBLEND, D3DBLEND_INVSRCALPHA);



Something like this could clean up your code.. some other things where mentioned in that discussion..

Has anyone else read it? I can't find it anymore :(

Share this post


Link to post
Share on other sites
Quote:
Original post by Evil Steve
Quote:
Original post by Scheermesje
But what do you guys think of something like:
Renderer->getDevice()->SomeFunction ();

There is no set function for my device so it shouldn't be public but code with lots of those render->getdevice statements is looking terrible. Is there a solution for this?

I use code like that too. Its a lot better than exposing the m_pDevice (or whatever you call it) member variable, since using a GetDevice() stops someone from setting the pointer to something else and breaking the code. Effectively, its read only.
I don't think it looks ugly at all, but thats just my opinion


So the Renderer isn't adequate enough to do what you want without you accessing its implementation details. Sounds like a problem with your design.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!