|
No, it's the language with the most consistent function behaviour - php.
|
|
|
|
|
Nice one today:
'Not as strange as it looks, the property set adds the object to session
MemberData = MemberData
you know what? It really works.
|
|
|
|
|
Maybe that should be in the constructor?
|
|
|
|
|
I don't know what language that's in, but I'm wondering if relying on the side-effects of assigning a member to itself is a good idea. A compiler (assuming this is compiled code) might reasonably optimize that away.
However, you have to like the comment!
|
|
|
|
|
I assume its C# and a property. Then its a normal method call for the compiler.
|
|
|
|
|
comments with ' ... looks like VB or so ...
|
|
|
|
|
|
Well ok.
But even if its VB.Net its still the same.
On the other hand its total crap to abuse a property in this manner.
|
|
|
|
|
Robert Rohde wrote:
On the other hand its total crap to abuse a property in this manner.
Although such code should never be used outside a class, since it relies upon the class not to implement caching behavior (or else relies upon some class-specific means of overriding such behavior), setting a property to itself may be a reasonable way of forcing a refresh of a corruptible copy of a property from a stable one.
For example, one might have an object which manages a USB-connected lighting controller. Setting the "Brightness" property of the object will send a command to the controller to change the brightness. If external factors might disrupt the brightness setting of the external controller, it may be desirable to periodically set the brightness to the value the computer expects. If the property-set routine simply wraps a SetHardwareBrightness method, it would probably be better to call that method than set the property to its present value. If the property-set routine does the actual work itself, however, it's probably better to use the property-set routine for the refresh than to split out a new SetHardwareBrightness method.
|
|
|
|
|
Now if that were C++ and I had also written a copy constructor for the class, I'd be able to introduce some side effects that would be truly humourous. Assign NULL to some pointers, initialize some other variables. Probably make it to Coding Horrors.
Chris Meech
I am Canadian. [heard in a local bar]
In theory there is no difference between theory and practice. In practice there is. [Yogi Berra]
|
|
|
|
|
|
I try to remember to do a Find All Usages in Resharper (now in VS2010) to make sure that code inside the class doesn't use the property, only the backing field. Keeps me from writing code like that.
It goes without saying that any code outside the class shouldn't even have any idea what side-effects might be happening in a public setter.
Before .NET 4.0,
object Universe = NULL;
|
|
|
|
|
I found this pearl while doing a peer code review. This is not the actual code but it gave the "idea"...
typedef struct
{
bool m_bUsed;
} CONFIG;
#define MAX_CFG 10
static CONFIG g_vConfigurations[MAX_CFG];
...
bool IsUsed ( int iHandle )
{
if ( g_vConfigurations[iHandle ].m_bUsed == true )
return true;
else
return false;
}
A logic genius... And notice the lack of argument value check before using iHandle in IsUsed() function .
Bye By(t)e
|
|
|
|
|
Yeah, I've done that before, then looked back at my own code and said: WTF?
CQ de W5ALT
Walt Fair, Jr., P. E.
Comport Computing
Specializing in Technical Engineering Software
|
|
|
|
|
That's pretty much the reaction I have to any code I write.
|
|
|
|
|
I don't really mind code of the form
if (condition)
return 1;
else
return 0;
While it certainly could be replaced by:
return (condition) != 0;
the first form makes clearer that what's being handled isn't just data, but control information. I don't like comparisons with 'true', though, unless there's a clear and explicit reason, which should be commented (e.g. 'If "condition" has an invalid value, it got glitched, so return zero to show failure.').
|
|
|
|
|
supercat9 wrote: While it certainly could be replaced by:
return (condition) != 0;
Why the extra condition, what's wrong with:
return condition;
It's time for a new signature.
|
|
|
|
|
Richard MacCutchan wrote:
Why the extra condition, what's wrong with:
The statement "return (condition) != 0;" is guaranteed to return either a one or a zero, regardless of the value of condition. If condition holds a value other than one or zero (whether or not it ever should), the statement return condition; would return that value.
Also, I forgot to mention a few more advantages of the "if" form:
- Although the posted skeletal example didn't include any comments, one may readily add a comment to distinguish the success and failure cases.
- The 'if' form is readily adaptable and remains readable regardless of whether condition code meanings in the called and calling functions are the same or opposite.
- Code and breakpoints may be added to the error- or normal-behavior case without changing the program structure.
Some people may make fun of the 'if' form, but I tend to like it.
|
|
|
|
|
Agreed; I merely assumed that condition is a boolean expression that will always return true<code> or false - which is, of course, what it should do.
It's time for a new signature.
|
|
|
|
|
Yes, as you said, the code snippet I've posted is not complete (As I told I've not posted the actual code) However the coder was NOT minding to use the 'if' form to ensure return 'true' or 'false' independently from what value is in m_bUsed (moreover this is guarantee to be 'ture' or 'false' only by the code that modify it).
A more rational implementation using the 'if; form could be the following:
bool IsUsed ( int iHandle )
{
if ( iHandle < MAX_CFG )
return ( true == g_vConfigurations[iHandle ].m_bUsed );
else
return false;
}
or (in a more 'compact' form someone can like):
bool IsUsed ( int iHandle )
{
retrun ( iHandle < MAX_CFG )? ( true == g_vConfigurations[iHandle ].m_bUsed ) : false;
}
Someone can complain about the logic of returning false when parameter is out of range instead of an exception but we can use them (that is a portion of code that recently ported to C++ from C using an "incremental" approach) and stating that "you cannot be using something you don't have" is not so wrong
Bye By(t)e
|
|
|
|
|
Simone Serponi wrote: And notice the lack of argument value check before using iHandle in IsUsed() function
Who needs to check the argument for a valid range? Hey, it's not my fault if you don't know how to properly use a function
I have no smart signature yet...
|
|
|
|
|
It looks to me like the author knew that they were writing bad code, and tried to hide the fact that they were using globals, which just made it all that much worse.
But we all had to start somewhere. Slapping a poorly written function over the global accessor could be an evolutionary step towards a static class interface that properly encapsulates the data.
|
|
|
|
|
That is exactly what he said when I've told him to add the argument check .
Bye By(t)e
|
|
|
|
|
I know this is an old thread, but being the pedant I am I can't resist.
None of the examples of the 'right' way to do it are optimal.
Remember that m_bUsed is already defined to be a bool.
Also notice iHandle is an int, which could be negative.
typedef struct
{
bool m_bUsed;
} CONFIG;
#define MAX_CFG 10
static CONFIG g_vConfigurations[MAX_CFG];
...
bool IsUsed ( int iHandle )
{
if (iHandle >= 0 && iHandle < MAX_CFG)
return g_vConfigurations[iHandle ].m_bUsed;
return false;
}
|
|
|
|
|
BOOL IsNegative(float val) {
char buffer[10];
char* ptr;
char* strptr;
ptr = buffer;
sprintf(ptr, "%f", val);
strptr = strstr(ptr, "-");
if (strptr == NULL)
return FALSE;
if (*buffer + (strptr-ptr) == '-')
return TRUE;
return FALSE;
}
A friend posted me this when he worked in the finance sector. I recreated it from memory and I'am sure it was slightly worse than this.
|
|
|
|