CodeSOD: Giving Up Too Late

"Retry on failure" makes a lot of sense. If you try to connect to a database, but it fails, most of the time that's a transient failure. Just try again. HTTP request failed? Try again.

Samuel inherited some code that does a task which might fail. Here's the basic flow:

bool SomeClassName::OpenFile(const CString& rsPath){int count = 0;bool bBrokenFile = false;while(!curArchive.OpenFile(rsPath)&&!bBrokenFile){ bBrokenFile = count >=10; if(bBrokenFile) { ASSERT(false); return false; } Sleep(1000); count++;}....}

Indenting as in original.

This code tries to open a file using curArchive.OpenFile. If that fails, we'll try a few more times, before finally giving up, using the bBrokenFile flag to track the retries.

Which, we have a sort of "belt and suspenders" exit on the loop. We check !bBrokenFile at the top of the loop, but we also check it in the middle of the loop and return out of the method if bBrokenFile is true.

But that's not really the issue. Opening a file from the local filesystem is not the sort of task that's prone to transient failures. There are conditions where it may be, but none of those apply here. In fact, the main reason opening this file fails is because the archive is in an incompatible format. So this method spends 11 seconds re-attempting a task which will never succeed, instead of admitting that it just isn't working. Sometimes, you just need to know when to give up.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!