Best Practice for Values Return C/C++

The latter convention is commonly used when you want to return an error code that tells you exactly what went wrong, the former convention is used when you're happy enough with just knowing if there was an error or not (e.g. return NULL if you couldn't load the texture). Note that in this case you can still make a separate function to retrieve the exact cause if you need.

So it really depends on how you want to handle errors, pretty much.

Almost every function in my engine is of the form:

error = function_name (arg1, arg2, arg3...);

The return value is always an error number. However, if all the function needs to return is a positive integer, you can return that in the return value.

Almost every function call is followed by:

if (error < 0) { return (error); }

... or sometimes some other code or function call might be tried before or instead-of the return(error); portion.

Overall, the following is common (though some elements are not required in many cases).

error = value = function_name (arg1, arg2...); // skip "value =" portion if only error values are returned
if (error < 0) {
free_resources (arg1, arg2, arg3...);
return (error);
}
// continue on with valid positive value

The above approach has proved the most convenient for me over many applications and libraries, after trying all sorts of alternatives. One equally viable approach, and possibly better because it is more consistent, is to never return anything but error values from functions, and remove all values in arguments. I didn't do that, but now and then think that's a better approach (because absolutely uniform in layout).

I prefer "Texture = LoadTexture(path)". It makes it crystal clear that the texture is an output, not an input. It also enables you to write more natural expressions, like using LoadTexture(path) as an argument to another function.

I'm a natural advocate of Stroppy's solution which corresponds better the the equivalent in pseudo code. errors can be handled via exceptions for better messages/codes.

especially now that there is std::move.

I agree with Stroppy and Lightness - returning the value that is expected result of a function (instead of some error) is the way to go. Returning error codes is one of most counterintuitive ways for dealing with things.

Of course it all depends on the case - in your case you just want to return Texture so go with your first suggestion, like:

std::shared_ptr<Texture> texture = textureCache.GetTexture("...");

I can hardly think of situation that would justify what was suggested above about using return value only for error code (and returning objects by assigning to reference). First of all, its quite hard to determine quickly what happens because you may have references that have different meaning: in, out, in/out. There is no special syntax to indicate it so you have to write a description. If you really need several error codes (which is IMO very rare and if you write code that requires it very often, something is wrong.. - there are cases where such coding is probably only way to achieve something, but I doubt that your case belongs there).

If you REALLY need to use this approach, at least:

• Make sure your references already tell your intentions, so if you're using reference as "in" only, make it const, if you use it for return value don't make it const:
  

  
  int LoadTexture(const string& name, const Type& type, Texture& texture)
  {
      if (load_from_file(name, type))
      {
          texture = <loaded texture>
          return SUCCESS;
      }
      return FAILURE;
  }
  

  Here references that are only for reading are marked const, while your return reference is not - its easier to know whats going to happen that way.

• Or use std::tuple and std::tie to return multiple values from a function (for example std::tuple where int is error code)

an alternative:

Texture* loadTexture(const wchar_t* name, ErrorCode* error = 0);

Actually I don't think it's even worth talking about error handling in this particular function's interface, because a situation where a built-in texture does not load simply should not happen. If it does happen in a packaged release, then the installation is broken and all bets are off. Just printing a diagnostic message to user and killing the game sounds like a good move at that point instead of propagating the error any higher.

In a situation where it's actually reasonable to expect problems with loading a file - like loading replay files that users are intended to copy manually between each other, and which could end up with messed up permissions which prevent reading - I think it would be a more reasonable approach to first try to open the file, check for success, and then pass the file *handle* to a loader function which is then guaranteed to succeed and doesn't need an error handling interface.

I generally stick with.

int DoSomething(); // return a single value

void DoSomething(int* const result); // multiple value

If you've never seen const in that specific position (between the pointer and name), it means you can change the data but not the pointer (barring any goofy tricks).

Hi,

What's is the best practice for values return in a function in C/C++ for Game Engines??

In C/C++ I can return one value with the sentence return.

Texture = LoadTexture(path);

or I can return the value on second via

LoadTexure(path, &Texture);

In both examples the variable Texture is get by the function LoadTexture.

What's is the better??

Thanks

When you return by reference, the value is directly written to the variable and you effectively avoid creating a temporary object that will be copied to your variable. The cost of creating that temporary object and the cost of the copy could be potentially be a performance hit. So to answer your question, it is good practice to return by reference wherever possible.

Small values, including built-in types, will be returned in a register, which is faster than a reference because that would force a write to memory. For larger objects there is the return value optimisation (http://en.wikipedia.org/wiki/Return_value_optimization). And in cases where the RVO is impossible, if the class has a move constructor the copy can be avoided that way.

Basically you can return by value in most situations without penalty.

Almost every function in my engine is of the form:

error = function_name (arg1, arg2, arg3...);

The return value is always an error number. However, if all the function needs to return is a positive integer, you can return that in the return value.

Combining the error number and return value into one is pretty confusing. I do not consider this a good idea.

Almost every function call is followed by:

if (error < 0) { return (error); }

... or sometimes some other code or function call might be tried before or instead-of the return(error); portion.

I'm going to go out on a limb here and assume that for some reason you're not using exceptions in your code base (or maybe this is C, not C++?). That often leads to code which is littered with error checking. In my opinion, this makes it difficult for people to grok the code, because the logic all mixed up with (often unnecessary) error checking.

Exceptions and proper use of RAII techniques would go a long way to improve things (if this is indeed C++).

Overall, the following is common (though some elements are not required in many cases).

error = value = function_name (arg1, arg2...); // skip "value =" portion if only error values are returned
if (error < 0) {
free_resources (arg1, arg2, arg3...);
return (error);
}
// continue on with valid positive value

There's a lot I disagree with here. That kind of "Check error, free resources and return from the middle of a function" is a code maintenance nightmare and a recipe for memory leaks.

@phil_t: For whatever reason, that code struck me as javascript, not C++. Especially this:

error = value = function_name (arg1, arg2...);   // skip "value =" portion if only error values are returned

Hi,

What's is the best practice for values return in a function in C/C++ for Game Engines??


In C/C++ I can return one value with the sentence return.

Texture = LoadTexture(path);

or I can return the value on second via

LoadTexure(path, &Texture);

In both examples the variable Texture is get by the function LoadTexture.

What's is the better??

Thanks

To return a value, I would use the former. To change the value/state of a variable I would use the latter.

You are a bit too enthusiastic about being critical!

In my message I too warned that returning a value that serves two purposes (error-number and return value) was non-optimal. Nonetheless, I'm not so terrified by different programming styles as you appear to be. Note that while the form error = value = funcname(args); is non-optimal for reasons we both agree about, it is not as bad as it might first appear, because subsequent lines will contain the error variable when dealing with the error value, and will contain the value variable when dealing with a non-error return value.

My programs are compiled with C++ to gain certain C++ features, but they are mostly C style code (a huge pile of functions). Nonetheless, your next comment seems a bit disingenuous. You can't avoid having some extra code (like try blocks or something) unless you simply avoid detailed error handling and just catch all errors (and thus cannot easily distinguish them any longer). Unless you have some sort of futuristic psychic programming language, handling most errors individually is not something you can ignore without causing yourself (the programmer) and your application unfortunate consequences. Furthermore, catching errors close to their source is very helpful during development. A programmer can indeed push error handling from different places in his application, but just ignoring most errors is rather lame.

As for my personal way of thinking about "errors" in the widest context, I prefer to think of "errors" and "exceptions" as being different. Sure, they are both forms of errors, but dealing with problems like "segment violation" separately has become very convenient for me, and often reduces the quantity and potential confusion of my code (to other programmers).

Another point about my way of dealing with errors you seem to ignore. In a word, I refer to "consistency". The way I mentioned to deal with errors is completely uniform throughout the application, and therefore extremely easy to understand when reading --- both for me, and for other programmers who might want to paw through my code.

As for "clutter", the code following those functions that can generate an error is most often if (error < 0) { return (error); }. This single line is not some horrible form of clutter. The only way to handle errors in less than 1 line of code is... to not handle errors at all, which is not good practice in my experience.

You simply claim without justification that my technique is a "code maintenance nightmare". I tried all sorts of other approaches over decades of programming, and none were anywhere near as easy to understand or debug. I think you should understand that every single one of us is able to habituate practices, then naturally find those practices "easier"... simply because we've already habituated them, and not because they are better given objective analysis. And even if you do perform an "objective" analysis, what aspects are considered "positive" or "negative" is often a trade-off or subjective. Therefore, I am not offended by other suggestions like you and other "close minded" or "authoritarian" folks are. I don't doubt that most approaches are strong in some ways, and weak in others. I think the original questioner should consider all the responses and choose for himself.

PS: Because my code follows such uniform patterns, I typically encounter a "memory leak" in my programs about once every 3 to 5 years. Annoying? Sure. A significant problem? No way. Most programmers I know encounter vastly more memory leaks in their programs.

Almost every function in my engine is of the form:

error = function_name (arg1, arg2, arg3...);

The return value is always an error number. However, if all the function needs to return is a positive integer, you can return that in the return value.

Combining the error number and return value into one is pretty confusing. I do not consider this a good idea.

Almost every function call is followed by:

if (error < 0) { return (error); }

... or sometimes some other code or function call might be tried before or instead-of the return(error); portion.

I'm going to go out on a limb here and assume that for some reason you're not using exceptions in your code base (or maybe this is C, not C++?). That often leads to code which is littered with error checking. In my opinion, this makes it difficult for people to grok the code, because the logic all mixed up with (often unnecessary) error checking.

Exceptions and proper use of RAII techniques would go a long way to improve things (if this is indeed C++).

Overall, the following is common (though some elements are not required in many cases).

error = value = function_name (arg1, arg2...); // skip "value =" portion if only error values are returned
if (error < 0) {
free_resources (arg1, arg2, arg3...);
return (error);
}
// continue on with valid positive value

There's a lot I disagree with here. That kind of "Check error, free resources and return from the middle of a function" is a code maintenance nightmare and a recipe for memory leaks.

You simply claim without justification that my technique is a "code maintenance nightmare".

You're right, I didn't justify it. I'll do that now.

With RAII techniques, returning from the middle of a function often isn't such a big deal (although I'd still argue it makes the function harder to understand). (The only time I do early returns is for basic parameter checking at the beginning of a function).

If you're manually freeing resources though, it means you need to make sure to free the right resources at every exit point. This is obviously easier to get right if there is a single exit point. Otherwise, it's easy for someone to come along and add new code that allocates resources, and then forget to update the "free" code in one of the exit points. That's a code maintenance problem.

If it's just you working on the code and you are very diligent, then this may not matter to you.

This is a bit off-topic, as the OP didn't ask about error handling but how to return from a function - but I wanted to address maxgpgpu arguments concerning error-handling. It seems nice at first, but when you start wondering about it - is there really a reason to return several errors when initializing and setting data to your VBO? Or when reading from terrain data volume to create a terrain mesh? Or when loading a texture? In all these cases, if the game is going to run properly it _has_ to succeed.

If you can't load texture, load default texture thats guaranteed to be loaded (or application won't start). If shader is missing - return some default shader and log error (plus this should be of course fixed for the release, because missing textures and shaders are no good). If you can't initialize VBO, something is very wrong and your application either has flaws that lead to this or its exceptional behaviour and should just result in exception thrown.

What I'm trying to say is that if your code is error-driven you will spend a lot of time returning various error codes from nested functions and having to handle them. What I found way more useful is to code in a way that errors basically don't occur (as an expected result) or are very rare. Use assert() and logging in debug mode to catch possible fatal errors that result from invalid program state and fix them so they do not happen, or are handled as they should. For something that can always happen but is exceptional throw exceptions. In rare cases you may need some multiple-value error code, but I looked through my app and I can't see anything like this (I don't count functions that don't return any other result than bool for success/failure).

I agree with phil_t. For C++ code that actually has to deal with errors, whether it does early returns or uses exceptions, the neat way to do cleanup is to have it happen automatically upon exiting the scope. For resources you have smart pointers and handles. For actions that have to be taken, there are scope guards.

This stuff should be familiar to all veteran C++ programmers. Here's a 13-year old article introducing scope guards:
http://www.drdobbs.com/cpp/generic-change-the-way-you-write-excepti/184403758
Of course these days they are even more convenient to use due to improvements in C++11.

That's quite right. Most error checking can be within an #ifdef DEBUG block so release builds don't contain extraneous error checking. But that doesn't mean applications should be developed without error checking. That would truly make building reliable applications difficult.

It is completely true that functions can often be written so they catch (and report or freeze or something) on all possible errors, and then don't return errors - and there's nothing wrong with that. My message above stated that not every function needs to return errors (which everyone seems to miss or ignore).

I'm not sure what "error driven code" is supposed to be. In my programs, including my 3D simulation/graphics/game engine, errors are extremely rare, pretty much vanishingly rare. You could say, this (and many programs) are "bomb proof" in the sense that they are rock solid and have no "holes". Unfortunately, things go wrong in rare situations with OS API functions and library functions, including OpenGL, drivers, and so forth... so even "a perfect application" needs to recognize and deal with errors.

I suppose there's nothing wrong with converting everything into exceptions, but after trying that and other approaches, I prefer mine. Nothing is fundamentally wrong with either - no single way is best in every respect. In these short messages there is too little space to discuss all the consequences of every kind of situation and pattern in every approach. Suffice it to say that the trade-offs are complex, multi-dimensional and not easy to explore in detail in compact messages.

This is a bit off-topic, as the OP didn't ask about error handling but how to return from a function - but I wanted to address maxgpgpu arguments concerning error-handling. It seems nice at first, but when you start wondering about it - is there really a reason to return several errors when initializing and setting data to your VBO? Or when reading from terrain data volume to create a terrain mesh? Or when loading a texture? In all these cases, if the game is going to run properly it _has_ to succeed.

If you can't load texture, load default texture thats guaranteed to be loaded (or application won't start). If shader is missing - return some default shader and log error (plus this should be of course fixed for the release, because missing textures and shaders are no good). If you can't initialize VBO, something is very wrong and your application either has flaws that lead to this or its exceptional behaviour and should just result in exception thrown.

What I'm trying to say is that if your code is error-driven you will spend a lot of time returning various error codes from nested functions and having to handle them. What I found way more useful is to code in a way that errors basically don't occur (as an expected result) or are very rare. Use assert() and logging in debug mode to catch possible fatal errors that result from invalid program state and fix them so they do not happen, or are handled as they should. For something that can always happen but is exceptional throw exceptions. In rare cases you may need some multiple-value error code, but I looked through my app and I can't see anything like this (I don't count functions that don't return any other result than bool for success/failure).

I have to disagree with part of your message. In many of the functions in my engine, some resources further down in the function cannot be created unless the resources created earlier in the function succeed. So you can't just willy-nilly "free everything" in one place... at least not in every function or situation. Some of these problems can be eliminated by setting all resource variables to zero at the function entry, and then checking those resource identifiers for non-zero values in a single "free all resources if an error is encountered" spot near the end. I used to do that sometimes years ago, but found that "freeing the appropriate resources" in each place is no big deal. I suppose part of the reason for that is that these situations occur only a handful of times in even my large applications.

One thing that I always try to point out is this. Over the years I have developed an extremely uniform and simple way of writing code. Pretty much every function looks the same as every other, and all share the same general practices. I admit this took quite a few years to figure out and solidify, but now that I've done that, I just don't have programming problems. Sure, I still have design problems... figuring out ways to solve a problem [cleanly or efficiently], but the coding part is now easy. What I'm trying to say here is... uniformity is probably more important than specific practices --- assuming we are choosing from a good solid set of alternatives of course.

BTW, though I write very large and complex applications, I freely admit that my coding style is very simple, straightforward and exceedingly consistent. I go out of my way to avoid mixing and matching multiple [fancy] approaches like many people do (and pride themselves in doing). This makes "being diligent" trivial and automatic (thoroughly habituated). My approach works great for me, but won't satisfy some others. For programmers who love complexity, and mixing and matching all sorts of approaches, I have nothing to suggest --- except "duck".

You simply claim without justification that my technique is a "code maintenance nightmare".

You're right, I didn't justify it. I'll do that now.

With RAII techniques, returning from the middle of a function often isn't such a big deal (although I'd still argue it makes the function harder to understand). (The only time I do early returns is for basic parameter checking at the beginning of a function).

If you're manually freeing resources though, it means you need to make sure to free the right resources at every exit point. This is obviously easier to get right if there is a single exit point. Otherwise, it's easy for someone to come along and add new code that allocates resources, and then forget to update the "free" code in one of the exit points. That's a code maintenance problem.

If it's just you working on the code and you are very diligent, then this may not matter to you.