Back to General and Gameplay Programming

Passing constructor call to function - Is this safe?

General and Gameplay Programming Programming

Started by InsaneBoarder234 July 22, 2006 10:51 AM

41 comments, last by owl 17 years, 8 months ago

1,729

July 22, 2006 03:07 PM

Quote:Original post by Mathematix
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.

If it's not good coding practice, it's not very likely to be the "best" option now is it? After all, good coding practice is all about solving problems in a best manner possible.

Quote:
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!

You stated the problem, as mikeman points out, as being a hard to find bug (lies - use a debugger) deriving from the possibility of the pointer being NULL. As I just said, global pointers suffer this exact same possibility. As such, your "solution" does nothing to alieviate the indicated problem. Is that explicit enough?

Quote:
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?

At this point, I'm lost as to what you're trying to describe. Post some code to illustrate what you mean? Did you mean "pointer-to-global" instead of "global pointer" perchance?

Quote:
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.

What wasn't described is a situation where multiple instance of SomeClass are not required. Given that we're dealing with abstracts such as SomeClass, Foo, Bar, etc - it'd be insane not to think we need to address the general case - which, if the standard library is any judgement, consisting 99%+ of classes that are not limited to one instance - would seem to be "any number of instances" rather than "one, only one, and nothing more than one instance".

Quote:
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. :)

When justified. Justification that seems to be lacking, in this case. The entire proposal makes me go "What? Why? You want to add a bunch of assumptions to our code (specific global variables instead of any old variable the client wants to pass along as an argument), and the only apparent justification falls completely flat (that it somehow magically makes it unable to be NULL, less likely to be NULL, or anything else along those lines).

Hence the reaction. Adding globals without answering the "why".

Here's a good why: My program depends on this logger in multiple places throughout the code, and will not need multiple loggers in this architecture's forseeable future (it's a prototype or the program will need a full rehaul of architecture anyways if our original requirements have changed that drastically). As such, the simplicity of a global over dependancy injection (or alternative method) outweighs the risk of having to recode that assumption.

Mathematix

259

July 22, 2006 03:29 PM

Quote:Original post by mikeman
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.

You guys are completely missing my point. The original source was:

class SomeClass{    //...};void SomeFunc(SomeClass *pObj){    //...}int main(){    SomeFunc(&SomeClass());    return 0;}

Now remember here that we do not know the definition for SomeFunc(), and since we do not know that, you cannot claim that the code that you've provided is any more safe than mine.

Quote:Original post by mikeman
Your alternative obviously doesn't solve the problem you described(NULL pointers).

My solutions were suggestions on a viable solution to having multiple pointers to a single object. Surely you must see the logic in that?

Quote:Original post by mikeman
"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.

Given that a pointer can be NULL and a reference can't, there is no point in risking the potential for this to occur. UIf your pointer fails to be allocated in one place then you can introduce code to handle the exception at that point, rather than waste time going any further down the pipeline.

Quote:Original post by mikeman
Communicating with the function via a global pointer is completely unreasonable and solves absolutely nothing.

Ummm, okay. If you say so.

Quote:Original post by mikeman
You're just substituting the argument list with a global variable, and nothing more.

And that was my intention.

Quote:Original post by mikeman
There's absolutely no reason to do that, and about a dozen reasons not to do it.

And the dozen reasons are?

You do realise that even though you are objecting, my suggestions will still work. ;)

rip-off

11,000

July 22, 2006 03:40 PM

Yes, your suggestions will work. But they are not the best idea, IMO.

Quote:
Quote:
You're just substituting the argument list with a global variable, and nothing more.

And that was my intention

You have yet to prove what you gain from that.

The problem here is the lifetime of the temporary in main. The calling code need not be modified, the problem applies to any function taking a temporary variable reference.

IMHO ([smile]) my way (with references, const references if possible ) is the simplest solution.
Can you give me any reason why this is not the case?

blog | twitter

mikeman

3,017

July 22, 2006 03:41 PM

You've completely lost me. First you provide alternatives you claim solve this problem:

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.

...then you say that those alternatives do not solve the above problem at all but are a "viable solution" for handling multiple pointers to the same object, then you talk about references that can't be NULL whereas minutes ago you were talking about global pointers. I can't really understand what you're trying to solve here. Catching NULL pointer *is* a problem, but your alternatives don't solve that. Other than that, all we know is that we deal with a function that takes a pointer to an object as an argument. We know nothing more, so I *really* don't understand what kind of problem we're trying to solve here.

Quote:
You do realise that even though you are objecting, my suggestions will still work. ;)

Actually, now I don't even realize what your suggestions are. Global pointers,references, what? Perhaps you could just write the code of your solution so we can see what you mean?

By the way:

Quote:
Now remember here that we do not know the definition for SomeFunc(), and since we do not know that, you cannot claim that the code that you've provided is any more safe than mine.

I haven't provided any code at all. The example I gave just demonstrated when the temp objects are created and destroyed.

Hyperdrive - Futuristic Combat Racing

rip-off

11,000

July 22, 2006 03:46 PM

Quote:
Now remember here that we do not know the definition for SomeFunc(), and since we do not know that, you cannot claim that the code that you've provided is any more safe than mine.

Yes we can, provided there are no globals interacting with the function. [smile]

The function, taking a reference to a SomeClass, cannot store the address of a temporary unless it makes use of some global. So by omitting globals entirely, and seeing as there are no other parameters which could potentially be used to store a reference to the temporary, there can be no trailing references. This is perfect.

Your method introduces a global, which casts doubt. If the global is only used for communicating an argument to the function, then why isnt it an argument in the argument list?

The "solutions" you posted ( all of them contain globals? ) only introduce doubt on what would otherwise be a safe function for temporaries.

blog | twitter

Mathematix

259

July 22, 2006 03:46 PM

Guys

You have offered your solutions and I have offered mine. I'm not in the mood for an ego trip. Different programmers do things different ways.

mikeman

3,017

July 22, 2006 03:50 PM

Quote:Original post by Mathematix
Guys

You have offered your solutions and I have offered mine. I'm not in the mood for an ego trip. Different programmers do things different ways.

But what *is* your solution? What *is* the problem you're trying to solve? Nobody is in an ego trip, we just don't understand what you're saying. Just post a short snippet of your #2 solution.

Hyperdrive - Futuristic Combat Racing

Mathematix

259

July 22, 2006 04:23 PM

class SomeClass{    //...};void SomeFunc(SomeClass *pObj)	// This function which is not a member of SomeClass{								// expects a valid pointer to     return;}int main(){	SomeClass *anInstance;    SomeFunc(anInstance);	// Pass uninitialised pointer to SomeFunc(). Code bombs-out at runtime.    return 0;}

The above can simply be fixed by simply having

class SomeClass{    //...};void SomeFunc(SomeClass *pObj)	// This function which is not a member of SomeClass{								// expects a valid pointer to     return;}int main(){	SomeClass *anInstance;	try	{		anInstance = new SomeClass;	}	catch (...)	{		cout << "Could not allocate instance of class" << endl;		exit(1);	}    SomeFunc(anInstance);	// Pass uninitialised pointer to SomeFunc(). Code runs fine.    return 0;}

Depending on what you want to do you can declare anInstance locally within main() or globally.

It's simple and it works if you use it carefully.

mikeman

3,017

July 22, 2006 04:46 PM

????

That's a valid solution(well, the use of exceptions here doesn't seem exactly right, but whatever), but you don't communicate with the function via a global. You do it properly like everyone else proposed, via an argument. Exactly like I said some posts ago:

Quote:
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

We were arguing all along about using globals, and now you present a solution that doesn't use one?

Hyperdrive - Futuristic Combat Racing

Mathematix

259

July 22, 2006 04:49 PM

Quote:Original post by mikeman
We were arguing all along about using globals, and now you present a solution that doesn't use one?

I have presented a solution that can use one, depending on the implementation!

Passing constructor call to function - Is this safe?

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Passing constructor call to function - Is this safe?

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines