Back to General and Gameplay Programming

Opinion needed: returning values without null checks everywhere

Marko Pipal · 2014-05-14T18:24:16

Hi all. Here at work, we have a class that stores non-POD objects by raw pointer, and has a get method that returns that raw pointer if the key was found (the storage is an std::map<key, value*> and the get method has a signature value* getThing(const key&); ). The objects are created elsewhere and simply stored in this class for ownership. A note to keep in mind is that c++ exceptions are disabled, and we're not using c++11 yet. My issue with this is that everywhere the getThing method is called, we need to check for null, since if the value is not found, null is returned. This annoys me to hell, since it bloats the code everywhere with null checks. I have a few ideas about changing it. 1. Add a 'bool exists(const key&)' method which does the obvious, and change the getThing to return a reference. - The issue here is if someone calls getThing without checking if the value exists, then i don't know what to return? The .second value of the end iterator? It should cause a crash as soon as someone uses the reference, right? - Since exceptions are disabled, i can't throw in case nothing was found, so i'm forced to always return something, and end.second seems like the only choice. - Another problem with this is that the getThing method is const too, so i'm not sure if returning a plain reference will work (i remember it being a compilation error [in some cases]) or if the reference also has to be const, which makes it impossible to change the thing we get. A solution is to remove the const, but isn't nice. 2. Have the getThing method return a bool, and accept a double pointer or reference-to-pointer as an 'out' argument, so if the value is found, true is returned and the out argument is set to point to the found value. - This seems ok and removes the need for a null check at the call site, but is again using pointers instead of references. I don't have anything against pointers, mind you, but i believe having references in the public interface of a class makes the code look cleaner since it's similar to the rest of it (using references everywhere instead of a mix of pointers and references, i find pointers fine for internals of specific classes). I'd like to get others opinion on this, another perspective on how this could be done better and/or different. For all i know, maybe the current implementation is fine, and i just need to come to terms with it... Any suggestions are welcome. Thanks!

General and Gameplay Programming Programming

Started by Strewya May 13, 2014 08:19 AM

21 comments, last by Aardvajk 9 years, 11 months ago

SeraphLance

2,613

May 14, 2014 02:42 PM

Maybe I'm missing something, but I don't see how this is all that bad.

If you want terseness (though I don't particularly like this sort of code) you can just do this:


if(value *foo = getThing(someKey))
{
  //do stuff
}

As far as I know, your only options for dealing with Hash Tables are NULL results, exception throwing, an out parameter and success return (like TryGetValue in C#'s Dictionary), and optional types. All of them (sans exceptions, which is just ugly) are pretty much the same thing in the end.

I'm with frob that calling an "Exists" function first is a waste of both your time and the CPU's. This is also sort of similar to the EAFP principle that python programmers love so much. Take the Nike approach and "just do it".

TheComet

3,927

May 14, 2014 03:48 PM


if(value *foo = getThing(someKey)) { //do stuff }

I can't get that to compile here... I need to declare foo outside of the if clause:


value* foo;
if(foo = getThing(someKey))
{
// do stuff
}

Or do something extremely retarded, such as:


for(value* foo = getThing(someKey);foo;foo=0)
{
    // do stuff
}

Other than that, I agree that there's no problem with checking for NULL.

"I would try to find halo source code by bungie best fps engine ever created, u see why call of duty loses speed due to its detail." -- GettingNifty

Aardvajk

13,217

May 14, 2014 06:24 PM

I can't get that to compile here...

Then there is something up with your compiler. Declaration inside if statement of the form:

if(Type *t = whatever())
{
}

is perfectly standard. One rational for it was for things like:

if(Type *t = dynamic_cast<Type*>(p))
{

// t is only scoped where it is valid

}

journal | website | twitter | youtube

Opinion needed: returning values without null checks everywhere

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Opinion needed: returning values without null checks everywhere

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines