Best Practice for Values Return C/C++

Started by
25 comments, last by Lightness1024 11 years, 2 months ago
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.
Advertisement

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

Beginner in Game Development?  Read here. And read here.

 

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.

Beginner in Game Development?  Read here. And read here.

 

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


Where are we and when are we and who are we?
How many people in how many places at how many times?
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.

This topic is closed to new replies.

Advertisement