It seems pointless #define function

Question

I met some code in the lines:

BOOL CBlahClass::SomeFunction(DWORD *pdw) { RETURN_FALSE_IF_FILE_DOESNT_EXIST //the rest of the code makes sense... //... } 

Everything I see makes pretty good sense, except that I have a little question about the RETURN_FALSE_IF_FILE_DOESNT_EXIST line

I searched for this line and I found #define:

 #define RETURN_FALSE_IF_FILE_DOESNT_EXIST \ if (FALSE==DoesFileExist()) return FALSE; 

My question is: what the hell? Is there a good reason to make #define this way? Why not just write:

 BOOL CBlahClass::SomeFunction(DWORD *pdw) { if ( FALSE == DoesFileExist() ) return FALSE //the rest of the code makes sense... //... } 

The only reason I can do this is a little easier and a little less annoying to write out "RETURN_FALSE_IF_FILE_DOESNT_EXIST" and then write out "if (FALSE == DoesFileExist ()) return FALSE".

Does anyone see other reasons for this? Is there a name for this kind of thing?

Answer 1

Well, the idea of ​​using a macro is to simplify maintenance in situations where this particular check might change. When you have a macro, all you need to change is defining the macro, not traversing the entire program and changing each test individually. In addition, it gives you the ability to completely eliminate this check by changing the definition of the macro.

In the general case, when you have a fairly well-rounded repeating piece of code, it usually splits this code into a function. However, when this repeating piece of code needs to perform some unorthodox operation, such as return , which leaves the closing function, macros are basically the only option. (You can probably agree that such tricks should be reserved for the debug / service / system level code, and should be avoided in the application level code.)

Answer 2

This macro is most likely a defensive proposition that states a precondition. A prerequisite is that a certain file must exist, because it makes no sense to call a function if it is not. The author probably wanted the protection clauses to be noticeably different from the “regular” checks and make this a big, obvious macro. What this macro is here is just a style preference.

You write a sentence of protection when the condition is required to be true to continue. In a function called parseFile() , you can really expect the caller to check if the file exists. But in a function called openFile() , it may well be prudent that the file does not exist yet (and therefore you will not have protection).

You may be more used to seeing assert(...) . But what if you don't want your program to stop when the condition is false, and you want the condition to be checked even in the presence of NDEBUG ? You could implement a macro like assert_but_return_false_on_fail(...) , right? And what is this macro. Alternative to assert() .

glib is a very popular library that defines its assertion statements this:

 #define g_return_if_fail() #define g_return_val_if_fail() #define g_return_if_reached #define g_return_val_if_reached() #define g_warn_if_fail() #define g_warn_if_reached 

The same code as yours in glib will be g_return_val_if_fail(DoesFileExist(), FALSE); .

The code you pasted might be better. Some problems:

• This is very specific. Why not make it general by any condition, and not just whether the file exists?

• The return value is hardcoded in the name.

• Further diagnostics are not performed. The glib functions record a fairly detailed error on failure, including a stack trace, so you can see a series of calls leading to a crash.

These problems make the macro seem dumb and pointless. That would have seemed less pointless.

Or it is possible that the author was obsessed with macros, and I read too much in it.

Answer 3

There is no reason for this. The usual way programmers write such macros is that they have code with multiple return statements (which should be avoided if possible, but sometimes it’s just not wise to avoid it). Example:

 // Bad code err_t func (void) { err_t err; ... if(err == error1) { cleanup(); return err; } ... if(err == error2) { cleanup(); return err; } cleanup(); return err; } 

To clear the whole function, this is bad practice due to code repetition.

An ancient way to solve the problem would be purer "goto error", where you put a label at the end of the function. And do the cleaning and return only there. In fact, this will be a somewhat acceptable solution, but goto is a taboo, so people shy away from it. Instead, they come up with something like:

 // Slightly less bad code #define RETURN_FROM_FUNCTION(err) { cleanup(); return (err); } err_t func (void) { err_t err; ... if(err == error1) { RETURN_FROM_FUNCTION(err); } ... if(err == error2) { RETURN_FROM_FUNCTION(err); } RETURN_FROM_FUNCTION(err); } 

This removes code repetition, so the program is more secure and easy to maintain. This is probably the answer to your question.

But this macro is still not a good solution, because it makes the code more difficult to read, because C programmers read C well, but poorly read "native language, secret macro language." And, of course, there are common problems with macros: type of security, hard to debug / maintain, etc.

However, all of the above methods (including on-error-goto) are due to the inability to think outside the field. The optimal solution is as follows:

 // proper code err_t func (void) { err_t err; allocate(); // allocate whatever needs to be allocated err = func_internal(); cleanup(); // one single place for clean-up return err; } static err_t func_internal (void) // local file scope, private function { err_t err; ... allocate_if_needed(); // or maybe allocation needs to be here, based on results ... if(if(err == error1) { return err; } ... if(err == error2) { return err; } return err; }