Seemingly pointless #define of function

Question

I've encountered some code along the lines of:

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 I have a little question about the line RETURN_FALSE_IF_FILE_DOESNT_EXIST

I searched for this string and I find a #define:

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

My question is... what the hell? Is there any good reason to make a #define like this? 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 think of to do this is that it is a little bit easier and a little less annoying to write out "RETURN_FALSE_IF_FILE_DOESNT_EXIST" then to write out "if (FALSE==DoesFileExist()) return FALSE".

Anyone see any other reason to do this? Is there a name for this sort of thing?

Answer 1

Well, the idea behind using the macro is to simplify the maintenance in situations when this specific check might change. When you have a macro, all you have to change is the macro definition, instead of crawling through the whole program and changing each check individually. Also, it gives you the opportunity to completely eliminate this check by changing the macro definition.

In general, when one has a fairly well-rounded repetitive piece of code, one typically separates that code into a function. However, when that repetitive piece of code has to perform some unorthodox operation, like return that exits the enclosing function, macros are basically the only option. (One'd probably agree that tricks like that should be reserved to debugging/maintenance/system-level code, and should be avoided in the application-level code.)

Answer 2

That macro is likely a guard clause that asserts a precondition. The precondition being that a certain file must exist because it doesn't make sense to call the function if it doesn't. The author probably wanted guard clauses to be noticeably different than "regular" checks and made it a big obvious macro. That it's a macro here is simply a style preference.

You write a guard clause when you require a condition to be true to continue. In a function called parseFile(), you might indeed expect the caller to have checked that the file exists. But in a function called openFile() it could be perfectly reasonable that the file does not exist yet (and therefore you wouldn't have a guard).

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 checked even in the presence of NDEBUG? You could implement a macro like assert_but_return_false_on_fail(...), right? And that's what this macro is. An alternative assert().

glib is a very popular library that defines its precondition assertions like 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 would be g_return_val_if_fail(DoesFileExist(), FALSE);.

The code you pasted could be better. Some problems are:

• It is very specific. Why not make it generic over any condition and not just whether a file exists?

• The return value is hard-coded into the name.

• It doesn't do further diagnostics. The glib functions log a rather detailed error on failure, including a stack trace so you can see the series of calls that led to the failure.

These problems are what make the macro seem silly and pointless. It would appear less pointless otherwise.

Or it's possible that the author was macro-obsessed and I'm reading too much into it.