Passing constructor call to function - Is this safe?

Started by
41 comments, last by owl 17 years, 8 months ago
I just created a quick program to check for safe execution of this code and it seems safe (using std::cout to follow program flow), but I wanted a second opinion on it.

class SomeClass
{
    //...
};

void SomeFunc(SomeClass *pObj)
{
    //...
}

int main()
{
    SomeFunc(&SomeClass());
    return 0;
}

Is the behaviour of this code defined by the C++ standards? Or should I be creating the SomeClass object either by having "SomeClass someclass;" or "SomeClass *psomeclass = new SomeClass();" at the start of main()?
Progress is born from the opportunity to make mistakes.

My prize winning Connect 4 AI looks one move ahead, can you beat it? @nickstadb
Advertisement
SomeClass() creates a temporary object that is destroyed after SomeFunc() returns. It is safe to use it only inside SomeFunc(), but it's not safe to store the pointer "pObj" for later use. You can see how this work by observing how objects get created/destroyed:

class Foo{public:	int x;	Foo(int x):x(x)	{		cout<<"Foo object created"<<endl;	}	virtual ~Foo()	{		cout<<"Foo object destroyed"<<endl;	}};void Bar(Foo* foo){	cout<<"foo.x = "<<foo->x<<endl;}int _tmain(int argc, _TCHAR* argv[]){	Bar(&Foo(23));	cout<<"Next line after Bar()"<<endl;	return 0;}


You see that the output is:

Foo object created
foo.x = 23
Foo object destroyed
Next line after Bar()
Don't really see why you're doing this as you could be introducing one of those hard to find bugs. For void SomeFunc(SomeClass *pObj) you are passing an instance of SomeClass that can be NULL. Unless you have provided a check for this NULL instance of SomeClass and it does turn out to be NULL your program will crash. There are a number of alternatives:

1. Create a global pointer to some class and get it to0 locally point to an instance of this same class in your function. This is also bad practice, since the instance in your function will go out of scope and you global pointer will become undefined.

2. Create the aforementioned global pointer to SomeClass and use it in your functions as required. This appears to be the best option for what you want to do.

3. Have a pointer to SomeClass declared locally in your function and have it point to the global pointer/non-pointer instance. I see little reason for this implementation.

That's my two pennies! :)
Quote:Original post by Mathematix
Don't really see why you're doing this as you could be introducing one of those hard to find bugs. For void SomeFunc(SomeClass *pObj) you are passing an instance of SomeClass that can be NULL. Unless you have provided a check for this NULL instance of SomeClass and it does turn out to be NULL your program will crash. There are a number of alternatives:

1. Create a global pointer to some class and get it to0 locally point to an instance of this same class in your function. This is also bad practice, since the instance in your function will go out of scope and you global pointer will become undefined.

2. Create the aforementioned global pointer to SomeClass and use it in your functions as required. This appears to be the best option for what you want to do.

3. Have a pointer to SomeClass declared locally in your function and have it point to the global pointer/non-pointer instance. I see little reason for this implementation.

That's my two pennies! :)


Or you could try this:

class SomeClass{    //...};void SomeFuncByPointer_preferToUse_SomeFuncByReference_instead(SomeClass *pSomeClass){    // use *pSomeClass}void SomeFuncByReference(SomeClass &someClass){    // use someClass}int main(){    // code here    { // inner scope!         SomeClass temp;         SomeFuncByReference(temp);          SomeFuncByPointer_preferToUse_SomeFuncByReference_instead(&temp);    } // watch temp disappear!    // more here    return 0;}


We could also make the functions const if they do not modify their argument ( and seeing as you are happy to pass in a temporary, they probably don't ).

Thats just how I would do it, passing the address of a temporary gives me a warning so I would avoid it even if it is guarenteed to work.
Quote:Original post by InsaneBoarder234
Is the behaviour of this code defined by the C++ standards?


It isn't, as you cannot take the address of a temporary variable (you may only take the address of an lvalue). The result is either undefined behavior or outright compiler complaints.

It is, however, defined by one of the Visual C++ extensions to the C++ languages, which allows such use of temporary variables. Thus, your code works but is not portable.

Quote:Original post by Mathematix
...
2. Create the aforementioned global pointer to SomeClass and use it in your functions as required. This appears to be the best option for what you want to do.
...


Oh HELL no.

1) Global pointers can be just as NULL as function argument pointers, so your proposal dosn't solve the problem you described.
2) Global pointers can be modified from ANYWHERE, and as such, it can be harder to figure out what, for example, might be modifying the pointer in question to become NULL in the first place, so it adds to the problem you described.
3) That's not going to work if there's more than one SomeClass.
4) Oh HELL no.

OP: If SomeFunc allways operates on an object, consider using a reference instead ala:

void SomeFunc( SomeClass & Obj ){    //...}int main(){    SomeFunc( SomeClass() );}


Of course, if it's operating on a temporary, chances are it shouldn't be modifying the variable (since any changes would be immediately discarded), so a reference-to-const might be more appropriate:

void SomeFunc( const SomeClass & Obj ){    //...}int main(){    SomeFunc( SomeClass() );}


The only way for a reference to become null is through dereferencing a null pointer, which being undefined behavior according to the C++ standard, means you can't have null references in a well formed program. Generally, one should check to ensure the pointer is non-NULL before dereferencing it into a reference.
Quote:Original post by MaulingMonkey
Quote:Original post by Mathematix
...
2. Create the aforementioned global pointer to SomeClass and use it in your functions as required. This appears to be the best option for what you want to do.
...


Oh HELL no.

1) Global pointers can be just as NULL as function argument pointers, so your proposal dosn't solve the problem you described.
2) Global pointers can be modified from ANYWHERE, and as such, it can be harder to figure out what, for example, might be modifying the pointer in question to become NULL in the first place, so it adds to the problem you described.
3) That's not going to work if there's more than one SomeClass.
4) Oh HELL no.

OP: If SomeFunc allways operates on an object, consider using a reference instead ala:

void SomeFunc( SomeClass & Obj ){    //...}int main(){    SomeFunc( SomeClass() );}


Of course, if it's operating on a temporary, chances are it shouldn't be modifying the variable (since any changes would be immediately discarded), so a reference-to-const might be more appropriate:

void SomeFunc( const SomeClass & Obj ){    //...}int main(){    SomeFunc( SomeClass() );}


The only way for a reference to become null is through dereferencing a null pointer, which being undefined behavior according to the C++ standard, means you can't have null references in a well formed program. Generally, one should check to ensure the pointer is non-NULL before dereferencing it into a reference.


Notice that I said "This appears to be the best option for what you want to do", which is not the same as it being good code practice.

Quote:Original post by MaulingMonkey
1) Global pointers can be just as NULL as function argument pointers, so your proposal dosn't solve the problem you described.


Of couse, and I didn't imply otherwise. The point of making it global is that you will create an instance in one place and destroy the instance at anopther single place in the code.

You haven't pointed out why it wouldn't solve the problem!

Quote:Original post by MaulingMonkey
2) Global pointers can be modified from ANYWHERE, and as such, it can be harder to figure out what, for example, might be modifying the pointer in question to become NULL in the first place, so it adds to the problem you described.


What?! If you point to the global, as I was trying to get across, then make sure that you're not pointing to it again when leaving the function, why should that be a problem?

Again, how does this add to the problem?

Quote:Original post by MaulingMonkey
3) That's not going to work if there's more than one SomeClass.


You're pointing out a problem that was not described, in that multiple instances of SomeClass are required. If this was the case then I would have offered another solution via a design pattern.

You're quite right, though. I should have pointed out that this will cause problems for multiple instances of SomeClass.

Quote:Original post by MaulingMonkey
4) Oh HELL no.


Globals aren't necessarily a bad thing, and are used in production code when justified. What makes them appear bad is poor implementations elsewhere in the code. :)
Quote:Original post by Mathematix
Quote:Original post by MaulingMonkey
Quote:Original post by Mathematix
...
2. Create the aforementioned global pointer to SomeClass and use it in your functions as required. This appears to be the best option for what you want to do.
...


Oh HELL no.

1) Global pointers can be just as NULL as function argument pointers, so your proposal dosn't solve the problem you described.
2) Global pointers can be modified from ANYWHERE, and as such, it can be harder to figure out what, for example, might be modifying the pointer in question to become NULL in the first place, so it adds to the problem you described.
3) That's not going to work if there's more than one SomeClass.
4) Oh HELL no.

OP: If SomeFunc allways operates on an object, consider using a reference instead ala:

void SomeFunc( SomeClass & Obj ){    //...}int main(){    SomeFunc( SomeClass() );}


Of course, if it's operating on a temporary, chances are it shouldn't be modifying the variable (since any changes would be immediately discarded), so a reference-to-const might be more appropriate:

void SomeFunc( const SomeClass & Obj ){    //...}int main(){    SomeFunc( SomeClass() );}


The only way for a reference to become null is through dereferencing a null pointer, which being undefined behavior according to the C++ standard, means you can't have null references in a well formed program. Generally, one should check to ensure the pointer is non-NULL before dereferencing it into a reference.


Notice that I said "This appears to be the best option for what you want to do", which is not the same as it being good code practice.

Quote:Original post by MaulingMonkey
1) Global pointers can be just as NULL as function argument pointers, so your proposal dosn't solve the problem you described.


Of couse, and I didn't imply otherwise. The point of making it global is that you will create an instance in one place and destroy the instance at anopther single place in the code.

You haven't pointed out why it wouldn't solve the problem!


The problem, as you stated it, was:

Quote:
Don't really see why you're doing this as you could be introducing one of those hard to find bugs. For void SomeFunc(SomeClass *pObj) you are passing an instance of SomeClass that can be NULL. Unless you have provided a check for this NULL instance of SomeClass and it does turn out to be NULL your program will crash. There are a number of alternatives:


Your alternative obviously doesn't solve the problem you described(NULL pointers). "Creating an instance in one place and destroy it at another" is another problem entirely. Doesn't have anything to do with whether the pointer is NULL or not. In that case, you could just create an instance, pass it to the function as an argument, and destroy it at another place, like the OP said in hist first post:

Quote:
Or should I be creating the SomeClass object either by having "SomeClass someclass;" or "SomeClass *psomeclass = new SomeClass();" at the start of main()


Communicating with the function via a global pointer is completely unreasonable and solves absolutely nothing. You're just substituting the argument list with a global variable, and nothing more. There's absolutely no reason to do that, and about a dozen reasons not to do it.
Thanks for the replies all. It seems I was overcomplicating things a bit with my project and this code is unnecessary, sorry!

Either way, I have learnt something! It's probably best to use a const reference if I end up implementing something like this in the future. Thanks again for the replies.
Progress is born from the opportunity to make mistakes.

My prize winning Connect 4 AI looks one move ahead, can you beat it? @nickstadb
Beware global variables. Once upon a time they were nothing special, but in these days of multi-core machines and corresponding shift towards multithreaded code, they're notable as the bane of thread-safe programming.
"Walk not the trodden path, for it has borne it's burden." -John, Flying Monk

This topic is closed to new replies.

Advertisement