Jump to content
  • Advertisement
Sign in to follow this  
BaneTrapper

Returning a nullptr refence, how bad is my teammate?

This topic is 1254 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Does anyone find this code being very bad?

Can you rate this from 0 to 10. I am very interested what is your opinion on returning a reference that is nullptr.

const std::string &SoundHandler::getPlayingFromPlaylist(const std::string& playlistName)
{
    //Check to see if the playlist exists
    auto iter = playlists.find(playlistName);
    if(iter == playlists.end())
        return nullptr;

    return playlists[playlistName].currentlyPlaying;
}

Share this post


Link to post
Share on other sites
Advertisement
Ordinary, such code would not compile, but in this case I believe this will attempt to construct a temporary std::string (as if from a null character pointer) which most implementations will assert/guard against, but either way this an express trip through undefined behaviour land, likely destination Teh Crash.

Nice of them to test the function thoroughly though, in addition to compiling on an insufficient warning level (and without warnings as errors).

Share this post


Link to post
Share on other sites

I never meant it as an insult, i just couldn't force him to change it happy.png . I expect references to be valid, if he cannot ensure it, return a pointer.

After a laugh he did a quick fix cool.png , thx on the opinions.

EDIT:: The compiler gave a warning, he though its okay.

Edited by BaneTrapper

Share this post


Link to post
Share on other sites

In situations where it can optionally return nothing, I'd prefer something list std::optional / boost::optional.

 

What I actually do in these situations, is return a const reference to a static empty version and log a warning/error. Not ideal, but better than a lying reference. However, I've been deciding more and more that asserts are my friend.

Share this post


Link to post
Share on other sites

that code is creating a std::string temporary out of the null pointer (via the constructor that takes a const char *) and returning that.

I thought it would be something like that... Except isn't nullptr supposed to solve this problem (compared to NULL/0) where null pointers weren't typesafe and thus caused people to unknowingly call the wrong function?

(edit) brain fart - nullptr prevents it being passed as an int, but has to be able to implicitly convert to any/all pointer types...

Share this post


Link to post
Share on other sites

Returning nullptr there will invoke undefined behavior as nullptr does not point at an array of at least traits::length(s)+1 elements of CharT, which is a requirement of this overload.

 

There was probably good intent behind this (so I'm still giving 2 points rather than 0), assuming that std::string would make something meaningful (empty string, or "the null string") out of it, but it's really just undefined behavior.

 

(The reference to temporary thing isn't really much of an issue since it's const, so its lifetime is extended to the lifetime of the reference.)

Edited by samoth

Share this post


Link to post
Share on other sites

EDIT:: The compiler gave a warning, he though its okay.

 

That is why I love -werror or /WX. Reduces the chances of idiot programmers to ignore the warnings.

Edited by l0calh05t

Share this post


Link to post
Share on other sites

 


EDIT:: The compiler gave a warning, he though its okay.

 

That is why I love -werror or /WX. Reduces the chances of idiot programmers to ignore the warnings.

 

 

It's really easy to ignore warnings, or just miss them if you don't seem them.  Having warnings as errors can seem like a pain, especially if turned on more than halfway through a project, but it's totally worth doing.  (And it's best done from the beginning)

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!