Non private privacy

547

Author

July 05, 2006 08:59 PM

Quote:Original post by mikeman
Quote:Original post by Kest
Quote:Original post by mikeman
But all of these methods named Foo.GetX() don't return X. For example, you don't write "mypixelshader=device->GetPixelShader()". They write the result to an OUT variable that is supplied in the argument list. Thus, if we assume that Foo.Y() is a good practice and in this case Y=GetX, D3D follows the same principle, since it returns a value indicating whether or not the method succededed. That is, the value they return is related to GetX(the method) and not X(the state).

Whether you want to..

return ObjectAddress;

..or..

*ptr = ObjectAddress;
return;

..you can't seriously think that one makes it okay to use Get in the function name and the other doesn't? If so, I give up. I don't even want to know why.

What are you talking about? We're not talking about what goes inside the function, we're talking how it appears to the outside,ie whether it's better to do:

(1)mytransformation=foo.GetTransformation();

or

(2)mytransformation=foo.Transformation();

You used the D3D example and I just pointed out that it doesn't help your argument, since it doesn't behave like (1) at all.

It bahaves like 3:
(3)foo.GetTransformation(&mytransformation);
Which behaves exactly like 1. It just doesn't look the same.

Also, it's not my argument. Get/Set is used by the majority. Heck, point out any popular game library that doesn't use them. It's the elites that keep changing the rules, even when they don't need changed.

edit:

I believe you should name a function to best describe it's behavior. If the function is sending back data, 'Get' makes sense. When I use Set in a function, I use it because it describes the action that the object is performing. Would SetMyPantsOnFire be a bad function? Nope. Because MyPantsOnFire is clearly not data. When I use SetPosition, I'm typing Position as in the object's coordinates on the map, not a variable named Position.

[Edited by - Kest on July 5, 2006 9:59:16 PM]

Emmanuel Deloget

1,382

July 06, 2006 03:39 AM

'lo Kest,

Quote:Original post by Kest
I'd love to see someone try to improve these Set.. function names.

IDirect3DDevice9::SetPixelShader()
IDirect3DDevice9::SetClipPlane()
IDirect3DDevice9::SetClipStatus()
IDirect3DDevice9::SetMaterial()

I already talked about exceptions, and these are some of those exceptions. SetPixelShader() does nothing else that setting the pixel shader. Same for SetClipPlane(), SetMaterial() and so on. They change a single value/substate which is not modified by the call (ie if you GetMaterial() right after, you get the material you've just set (the SetMaterial() call still have to be valid, of course; you can't expect DX to correctly handle invalid pointers, for example)). They perfectly fit what I already said. If SetPixelShader() was also changing the material then I would be more reluctant but AFAIK it is not the case. These are good, semantically valid names (which also have the benefits of being idiomatic). There is not real need for improvement here.

Quote:Original post by Kest
And can anyone tell me why these functions are named any better?

IDirect3DDevice9::CreatePixelShader()
IDirect3DDevice9::CreateTexture()
IDirect3DDevice9::MultiplyTransform()
IDirect3DDevice9::BeginStateBlock()

Is Create a bad word? Multiply? Begin?

Never said that. They all begin with a verb, implying an action, which is quite good IMHO.

Quote:Original post by Kest
Also, it's not my argument. Get/Set is used by the majority. Heck, point out any popular game library that doesn't use them. It's the elites that keep changing the rules, even when they don't need changed.

Well, Ogre makes a large use of destroyable singletons - that doesn't mean that it is a good thing.

There is not a lot of correctly designed libraries, and wide acceptance of a library doesn't mean that its design is good (MFC or the Win32 API are good examples of strangely designed libraries which are widely used). There is one reference C++ library we all know of which contains very few set() methods: the standard C++ library. The majority of set() methods lies in <iomanip> and they change a single substate at a time (and they don't modify their parameter).

Regards,

-- Emmanuel D. [blog, in French] [blog, very bad googlized translation]

MaulingMonkey

1,729

July 06, 2006 05:11 AM

Quote:Original post by Kest
*Direct3D examples*

I'd have prefered Select_____ and Selected_____. To me, this makes it clearer that:
1) We're likely using a shallow reference (pointer) rather than copying
2) We're dealing with a highly transitory "current state" rather than setting a possibly more permanent class value.

Quote:And can anyone tell me why these functions are named any better?

IDirect3DDevice9::CreatePixelShader()
IDirect3DDevice9::CreateTexture()
IDirect3DDevice9::MultiplyTransform()
IDirect3DDevice9::BeginStateBlock()

Is Create a bad word? Multiply? Begin?

Create is fine - it's a keyword of the higher level concept of a factory.

MultiplyTransform I'm not enthused about - not due to Multiply, but due to treating the verb Transform as a noun. Converting it to the more appropriate MultiplyTransformation, and it becomes obvious the function is focusing on the property rather than object as a whole. Switching it to the plain Transform (used properly as a verb this time) and one realises that IDirect3DDevice9::Transform sounds somewhat weird. The class is a bit odd, a combination of manager and state machine - a violation of SRP (Single Responsibility Principle). It thus follows prehaps Transform() should be a member of another class - IDirect3DRenderingContext9::Transform, prehaps.

I'm unfamiliar enough with DirectX that I can't comment much about BeginStateBlock. "Begin" certainly seems appropriate in this case - it starts the logical action of recording (according to the docs), even if internally it just sets "pointer_to_recorder = new recorder" or even "recording = true" (it'd be stupid to use threads for such a task).

Marcus Speight

152

July 06, 2006 06:32 AM

Quote:Original post by mikeman
we're talking how it appears to the outside,ie whether it's better to do:

(1)mytransformation=foo.GetTransformation();

or

(2)mytransformation=foo.Transformation();

You used the D3D example and I just pointed out that it doesn't help your argument, since it doesn't behave like (1) at all.

To me (1) is better as Transformation() could be a setter as well as a getter.
e.g. foo.Transformation() = mystransformation;

Without actually looking a the code for the class it is fairly mute anyway.

I also don't like using setters for variables that take fundamental types.
e.g. SetColour(int); Why are we passing an int for a colour. Now passing a colour to the function makes more sense. Yes it could be a typedef. But then you can change it to a class without breaking code later.

Changing the colour of a GUI button in my mind is better done as a setter rather than an action. e.g. SetColour instead of Repaint. As I might not want to update the actual view of the button until later. As in my mind repaint will redraw the button immediately.

Marcus SpeightIf at first you don't succeed.
Destroy all evidence that you tried.

Kest

547

Author

July 06, 2006 02:15 PM

Quote:Original post by Emmanuel Deloget
I already talked about exceptions, and these are some of those exceptions. SetPixelShader() does nothing else that setting the pixel shader. Same for SetClipPlane(), SetMaterial() and so on. They change a single value/substate which is not modified by the call (ie if you GetMaterial() right after, you get the material you've just set (the SetMaterial() call still have to be valid, of course; you can't expect DX to correctly handle invalid pointers, for example)).

It's okay because they change one value, then return? And SetColor..? If you call GetColor right after you use SetColor, you might get the correct color back. But there's no written or unwritten law that states you should be able to do this anyway.

So you've got this state of mind where you believe that using Set in a function name implies that you're either modifying member variables or setting single values. I'm sorry to have to slide you over into the correct lane, but Set does not imply any such thing.

It's all in your mind. It's likely because you've either written so much code to perform this way, or seen so much of it. Please take a step back and look at it from outside of the small box. Get means you are trying to obtain something, and that is it. It does not mean you are trying to get the value of the object's member variables or internal states. Set means you want to make changes, or put something into place (ie-Direct3D). It does not mean you are breaking OOP laws and ripping the guts out of your objects.

You can choose to stop using these words if you really believe it keeps you from gutting your objects, but I don't need to do the same.

NotAYakk

876

July 06, 2006 04:00 PM

Zahlman, note that your property and member class should actually be more verbose. You should override most of the common operators (just in case they are needed), so people can do

foo.length *= 2;

Implementing a pseudo-ref is not that simple. Of course, you can implement a single pseudo-ref class, and it should serve 99% of your psuedo-ref needs.

Deyja

920

July 06, 2006 04:39 PM

NotAYakk; simple overriding the conversion operator will take care of most uses of the member - unless the usage was already relying on another user-defined conversion.

http://jemgine.omnisu.com

Zahlman

1,682

July 06, 2006 05:18 PM

I didn't even implement a conversion operator, but a call operator; that way, it "looks like property syntax", i.e., as if the data member were actually a member function of the same name, overloaded to return-data-by-reference (which allows whatever modification of course: 'foo.bar() += 5;') and set-data.

NotAYakk

876

July 06, 2006 05:49 PM

Exposing a raw reference to data means you are forced to store an actual instance of the data. About all you can do is log access, you can't do any mojo when the property is changed.

Heck, if you expose a T&, you can't even do sanity checking on the values set!

If you expose a T const&, you have to store a T somewhere, and the lifetime of that T has to be at least as long as the lifetime of any stored T const&s. So even a T const& gets in the way.

And if you don't expose a T&, you can't expect operator*= or operator+= to work without recoding it. You can, admittedly, simply implement it as x = x*y, but it won't work unless you at least do this.

This doesn't matter all that much, but it is an extra pile of boilerplate code you will want to write for any pseudo-reference class.

Don't get me wrong -- pseudo-references are useful, but they do require more than operator T() and operator=(T) to be fully functional.

Emmanuel Deloget

1,382

July 07, 2006 03:41 AM

Quote:Original post by Kest
Quote:Original post by Emmanuel Deloget
I already talked about exceptions, and these are some of those exceptions. SetPixelShader() does nothing else that setting the pixel shader. Same for SetClipPlane(), SetMaterial() and so on. They change a single value/substate which is not modified by the call (ie if you GetMaterial() right after, you get the material you've just set (the SetMaterial() call still have to be valid, of course; you can't expect DX to correctly handle invalid pointers, for example)).

It's okay because they change one value, then return? And SetColor..? If you call GetColor right after you use SetColor, you might get the correct color back.

So that's fine (according to my definition)

Quote:Original post by Kest
But there's no written or unwritten law that states you should be able to do this anyway.

So you've got this state of mind where you believe that using Set in a function name implies that you're either modifying member variables or setting single values. I'm sorry to have to slide you over into the correct lane, but Set does not imply any such thing.

Well, from dictionary.com:

Quote:
Set
2. To put into a specified state

Which lead me to :
1) If the set() method change its argument, you can't properly specify the state - thus you can't say that you set it.
2) If you specify a particular state or substate (such as the Position state) and modify something else (such as the boundary box) then you no longer specify the position itself, but something broader. SetPosition(), in this case would be misleading, and certainly don't obbey this definition.

Quote:Original post by Kest
It's all in your mind. It's likely because you've either written so much code to perform this way, or seen so much of it.

It is true - it is all in my head (I implied this numerous times in my previous posts, as you can verify by yourself). My experience is also talking for me (I'm a software architect, I do a lot of software design).

Quote:Original post by Kest
Please take a step back and look at it from outside of the small box. Get means you are trying to obtain something, and that is it. It does not mean you are trying to get the value of the object's member variables or internal states.

Well, obviously, it does. Set is an English word with a rather precise definition [smile]. Subverting the language you chose to program is not a good idea for readability. Pushing away abstraction which can come from your language is also not a good thing, as it restricts the thinking process.

Quote:Original post by Kest
Set means you want to make changes, or put something into place (ie-Direct3D). It does not mean you are breaking OOP laws and ripping the guts out of your objects.

If you obey some basic principles about the set() methods then you don't break anything. But if you don't then it will surely break some basic programming law ("find a good name for your identifiers" is not related to OOP).

Quote:Original post by Kest
You can choose to stop using these words if you really believe it keeps you from gutting your objects, but I don't need to do the same.

I use them - in very particular cases. But even if it is not enterily possible, I tend to avoid them. I (as in opinion) consider that my object internal state can't be known from the oustide, thus setting a substate don't have much sense.
Of course, you don't need to do the same. My goal is not to tell you what you should do or what you should not do, but to explain my thoughts on this particular subject. The cool thing is that it leads to a very interesting discussion [smile].

Regards,

-- Emmanuel D. [blog, in French] [blog, very bad googlized translation]

Non private privacy

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Non private privacy

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines