Is this a horror? [modified]
-
The
if (this != NULL)is not required and is just overkill; although I have personally seenthisequalNULLonce 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 thethispointer isNULL. 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.
Software Zen:
delete this;
Fold With Us![^] -
I'm an old-school 'C' programmer. When the language was originally defined,
NULLwas not guaranteed to be defined as(void *)0, although that was the typical definition. For that reason, I tend to useNULLrather than0, 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 forif (p != NULL)andif (p), so it doesn't affect performance anyway.Software Zen:
delete this;
Fold With Us![^]Re: Is this a horror?
Vishnu Rana Sr. Software Engg.
-
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.
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
MFCdevelopers 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 (
ASSERTit'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 -
The
if (this != NULL)is not required and is just overkill; although I have personally seenthisequalNULLonce 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
John R. Shaw wrote:
Now the fact that the method is not actually returning anything is much scarier.
Fixed :-O.
Software Zen:
delete this;
Fold With Us![^] -
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 aUserObjectis eitherNULLor identifies a valid object in the list. Theif (this != NULL)thing just creeps me out. This technique is used in MFC for the_class_::GetSafe_Handle_()functions, but that's not the most ringing endorsement.Software Zen:
delete this;
Fold With Us![^]modified on Thursday, May 15, 2008 5:26 PM
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
-
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!
Software Zen:
delete this;
Fold With Us![^] -
I'm an old-school 'C' programmer. When the language was originally defined,
NULLwas not guaranteed to be defined as(void *)0, although that was the typical definition. For that reason, I tend to useNULLrather than0, 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 forif (p != NULL)andif (p), so it doesn't affect performance anyway.Software Zen:
delete this;
Fold With Us![^]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.
-
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
MFCdevelopers 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 (
ASSERTit'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 ClarkeQuote: "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
-
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
BOOLis when it isn't really aBOOL. Some of the more ancient Windows API's returnBOOLvalues that can containTRUE,FALSE, or a piece of integer information :rolleyes:.Software Zen:
delete this;
Fold With Us![^] -
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.
MFCisC++or am I wrong (i.e.MFCdesigners useC++, 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 -
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.
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.