4 ms·
couldn't you just say: if(allocate_memory() != SUCCESS) { free_memory(); return error; } ... The other examples provided make a better use case of GOTO.
by kvinnako 14y ago
couldn't you just say:
if(allocate_memory() != SUCCESS)
{
free_memory();
return error;
}
...
The other examples provided make a better use case of GOTO.
- sophacles 14y agoWell, for starters the semantics of allocate_memory are (in general) such that a SUCCESS == false condition mean that no memory was allocated. So freeing the memory would be an error itself. This is me being a bit pedantic, but it is important to note that the error condition for resource allocation is generally "there is nothing from this step to deallocate". This can turn into an "off by one" error in cleanup if you aren't careful. Second, say your allocate_memory is successful... your next n steps look like: if (allocate_memory() != OK) { return error;} if (allocate_resource1() !=OK { free_mem(); return error;} if (allocate_resource2() !=OK { free_r1(); free_mem();} .... if (allocate_resourceN() !=OK { free_N-1();...;free_r2(); free_r1(); free_mem(); return error;} // .... CODE free_N(); free_N-1(); ... free_r2(); free_r1(); free_mem(); return success; There are two things to note here. First of all, the if lines get very long when you have to do all that unwinding at the loop. This is ugly and annoying, but itself not a problem. Second, and this is a code smell, you've gone and seriously repeated yourself. Now every time you add a resource allocation, you have to update the unwinding in several places. There is more room for error, more chance of subtle issues in order creeping in. etc. This pattern is a way to avoid it. The author addresses doing the whole chain in nested ifs in the original article.
- askimto 14y agoGenerally with that approach you can end up with a lot of duplicate code. It is more error prone.