|
class UserObject {
public:
static const unsigned DefaultLogID;
unsigned Log_ID();
private:
unsigned LogID;
};
const unsigned UserObject::DefaultLogID = 0;
unsigned UserObject::Log_ID()
{
unsigned log_ID = DefaultLogID;
if (this != NULL) {
log_ID = LogID;
}
return log_ID;
}UserObject's are stored in a linked list. A pointer to a UserObject is either NULL or identifies a valid object in the list.
The if (this != NULL) thing just creeps me out. This technique is used in MFC for the class::GetSafeHandle() functions, but that's not the most ringing endorsement.
modified on Thursday, May 15, 2008 5:26 PM
|
|
|
|
|
Well, I admit it looks ugly. Anyway since, at the end, an object's method is simply a function whoose first argument is a pointer to the object itself (i.e. this), the techinque appears (to me, at least) reasonable.
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
|
|
|
|
|
Let's start with the smallest problem. If you're paid based on number of characters you type then the line "if (this != NULL)" is perfect. Otherwise, it's enough to write just "if (this)".
But that's not what bothers you. You leave your readers in the dark, not everybody can guess your point. And your point is that it's absurduous to check the validity of the pointer INSIDE the function. Because that function is a class member, you can call it only having a valid pointer to an instance of that class. If the pointer is invalid, the call will fail and the execution of the function will not even begin.
More specifically, a call like pObject->LogID() will fail with ASSERT if pObject is invalid. The call will execute the function only and only if pObject is valid. To check later, in the function, the validity of pObject (locally known as "this") makes no sense.
But as I said earlier, if you're paid based on the number of characters you type, that line is perfect for you.
|
|
|
|
|
I'm an old-school 'C' programmer. When the language was originally defined, NULL was not guaranteed to be defined as (void *)0, although that was the typical definition. For that reason, I tend to use NULL rather than 0, even in C++. It's a harmless habit that improves readability of my code for me.
I've found that most compilers generate the same code for if (p != NULL) and if (p), so it doesn't affect performance anyway.
|
|
|
|
|
Gary R. Wheeler wrote: improves readability of my code
Yes, that's why I do it too. I feel that the lack of a distinct boolean type is one of the major drawbacks of C, it leads to obfuscation. The inclusion of a boolean type in C# (CLR languages) is one of the greatest strengths.
|
|
|
|
|
I believe that in C, typecasting or type-coercing a constant zero into a pointer is guaranteed to yield a null pointer, regardless of an architecture's actual storage of such things. Likewise, a null pointer is guaranteed to compare equal to a constant zero, and a non-null pointer is guaranteed to declare non-equal.
This does not imply that architectures have to actually store a value of zero for the null. It would be perfectly acceptable for an architecture to subtract one from an integer when casting to a pointer, and add one when casting back (so a null pointer would be stored as 0xFFFFFFFF). On some architectures there may be some significant advantages to doing so (e.g. if logical address 0x00000000 happened to be the start of user data space, or if an access to 0xFFFFFFFF would generate a hardware trap but 0x00000000 would not).
It's important to realize that C is actually very open-ended about certain implementation details. While the existence of bit-wise operators and the requirement that (-a)==(~a+1) would suggest that a machine that uses binary two's complement arithmetic would be desirable, it would nonetheless be possible to produce a fully-conforming implementation on hardware that used BCD for all math. The bit-wise operators would be extraordinarily slow on such a machine, but if the compiler's generated code produced correct results the implementation would be standards-compliant.
|
|
|
|
|
Re: Is this a horror?
Vishnu Rana
Sr. Software Engg.
|
|
|
|
|
I'm an old school programmer too, I still use NULL for handles and pointers and 0 for other numeric values but that's not the point.
"if(p)" is easier to write and also easier to understand instantly what it means.
I'm sure the generated code is the same because the code optimization will elliminate the pointless evaluation of the logical expression from "if(p == NULL)".
But if it may be still somehow acceptable for numeric variables it's absolutely absurduous when used on boolean variables. The result of the evaluation of the logical expression from an if statement is a logical value TRUE or FALSE.
Code sample:
BOOL b;
... use b ...
if(b == TRUE)
The if statement wants a boolean value, b without any strings attached to it, IS A BOOLEAN VALUE ready to be passed to that statement by writing simply "if(b)". Instead some programmers asks for an additional evaluation of a logical expression. Luckily the compiler is smarter and eliminates that nonsense during the code optimization.
|
|
|
|
|
The only time I've had to do a comparison on a BOOL is when it isn't really a BOOL. Some of the more ancient Windows API's return BOOL values that can contain TRUE, FALSE, or a piece of integer information  .
|
|
|
|
|
asrelu wrote: "if(p)" is easier to write and also easier to understand instantly what it means.
I'm sure the generated code is the same because the code optimization will elliminate the pointless evaluation of the logical expression from "if(p == NULL)".
I'm old school, too, and I respectfully disagree.
I use "if (p)" when p is used as a boolean. (It might actually be an int, because some of the old-school C that I help maintain). This is reminding me that p is a flag.
When I'm using a pointer, I use "if (p != NULL)". Sure, the compiler might optimize it to the exact same code as above but the content reminds me that p is a pointer.
Just my two cents. Your mileage may vary. This package is sold by weight, not by volume. You can be assured of proper weight even though some settling of contents normally occurs during shipment.
|
|
|
|
|
asrelu wrote: And your point is that it's absurduous to check the validity of the pointer INSIDE the function. Because that function is a class member, you can call it only having a valid pointer to an instance of that class. If the pointer is invalid, the call will fail and the execution of the function will not even begin.
More specifically, a call like pObject->LogID() will fail with ASSERT if pObject is invalid. The call will execute the function only and only if pObject is valid. To check later, in the function, the validity of pObject (locally known as "this") makes no sense.
You're wrong (after all MFC developers are skilled people). Try the following code:
class A
{
public:
void foo(){ printf("%p\n", this); }
};
int main()
{
A * p;
p = NULL;
p->foo();
}
BTW invalid pointers references cause runtime errors not assertions (ASSERT it's only a debug tool to intercept such occurrences).
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
|
|
|
|
|
Quote: "You're wrong (after all MFC developers are skilled people). Try the following code..."
You mix MFC with non-MFC, your sample has nothing to do with the MFC framework. I can only hope you'll not compile the release version without fixing an error signalled in the debug phase so I hope you'll not get a runtime error.
Thank you for your remark that
"invalid pointers references cause runtime errors not assertions (ASSERT it's only a debug tool to intercept such occurrences)".
In return, I want to offer you an advice having the same value:
If you want to fill a glass with water you shouldn't hold it upside down because the glass will not fill.
modified on Saturday, May 17, 2008 10:49 PM
|
|
|
|
|
asrelu wrote: Quote: "You're wrong (after all MFC developers are skilled people). Try the following code..."
You mix MFC with non-MFC, your sample has nothing to do with the MFC framework. I can only hope you'll not compile the release version without fixing an error signalled in the debug phase so I hope you'll not get a runtime error.
MFC is C++ or am I wrong (i.e. MFC designers use C++, do you realize)? My code simply shows that you assumption is definitely wrong.
asrelu wrote: Thank you for your remark that
"invalid pointers references cause runtime errors not assertions (ASSERT it's only a debug tool to intercept such occurrences)".
Don't be so upset, I fixed your terminology beacause it was wrong: nothing personal.
asrelu wrote: In return, I want to offer you an advice having the same value:
If you want to fill a glass with water you shouldn't hold it upside down because the glass will not fill.
Maybe it has the same value for you. For me it is simply a crap.
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
|
|
|
|
|
if (this) than that(); else damn (it);
Mostly, when you see programmers, they aren't doing anything. One of the attractive things about programmers is that you cannot tell whether or not they are working simply by looking at them. Very often they're sitting there seemingly drinking coffee and gossiping, or just staring into space. What the programmer is trying to do is get a handle on all the individual and unrelated ideas that are scampering around in his head. (Charles M Strauss)
|
|
|
|
|
The if (this != NULL) is not required and is just overkill; although I have personally seen this equal NULL once or twice in my career (probably a compiler dependent thing).
Now the fact that the method is not actually returning anything is much scarier. 
INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra
|
|
|
|
|
In my case, it's not an error check. The linked list is searched for a specific object. If the object is not found, the search returns NULL. The member functions in question return a default value if the this pointer is NULL. That way, the caller can do this:
obj->Value() instead of
if (obj != NULL) {
value = obj->Value()
}
else {
value = DefaultValue;
}every place he needs it.
|
|
|
|
|
John R. Shaw wrote: Now the fact that the method is not actually returning anything is much scarier.
Fixed  .
|
|
|
|
|
Class* obj = 0;
obj->Method();
The behavior of the call to Method() is not defined. It just happens that most (if not all?) implementations of the C++ language, implements methods like ordinary C functions, but with an added parameter: this.
Had the method been virtual, it would've crashed on the spot due to vtable lookups.
I'd say it's a code horror.
--
Kein Mitleid Für Die Mehrheit
|
|
|
|
|
Jörgen Sigvardsson wrote: Had the method been virtual, it would've crashed on the spot due to vtable lookups.
Aha!
I knew there was a concrete reason why this was a bad idea. I just couldn't think of it. Thanks!
|
|
|
|
|
Just tidying up some code I wrote when I started learning C#(some four months ago).
I found the following two horrors:
itemDups += 0;
goto Skip;
So I thought I better fess up and hang my head in shame.
Oh the shame...  
Continuous effort - not strength or intelligence - is the key to unlocking our potential.(Winston Churchill)
|
|
|
|
|
Well, not horrors:
GuyThiebaut wrote: itemDups += 0;
Simply exploiting sum neutral element.
GuyThiebaut wrote: goto Skip;
Time-travel to find out C# roots.
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
|
|
|
|
|
Embarassed? What for?
You will always find some interesting things when taking a close look at your older code. It simply means you have made some progress since then.
|
|
|
|
|
Thanks,
I think embarrassed because I have been coding for twenty years and I did not think I still wrote code like this.
You are right though , what is important is progress, we all have to start somewhere.
Continuous effort - not strength or intelligence - is the key to unlocking our potential.(Winston Churchill)
|
|
|
|
|
What shall I say? The little program I work on right now is a top candidate for this effect. The boss wanted a quick and dirty solution which will have to do until we come up with a better one. Along the way several things did not worke well and sometimes I had to take a step back and try something else.
The code is full of quick and dirty code to try out different approaches and also some remains of previous experiments. The whole structure and design has suffered from this tinkering. Now I'm cleaning it up a little, but still there will remain a lot to be improved. In a few weeks I will probaly plan the real solution and take this as a prototype. No doubt there will be many things which can be solved in a less error prone and better designed fashion.
But as I wrote, this little project has to be working quickly. In a few weeks it will have served its purpose and the encountered issues will help when planing the real solution. Quick and dirty is not always a bad thing. You just have to take it for what it's worth.
|
|
|
|
|
CDP1802 wrote: Quick and dirty is not always a bad thing
Definitely. But then you have to discard it and build the real one, exploiting acquired knowledge.
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
