Welcome to BSP Software’s ‘Code Review’ series
Peter Gabris, MD of BSP Software Inc
In this section we focus on
Part 4) Algorithmic soundness
- The execution should be final: a loop exit should be guaranteed, recursion should backtrack. An exception is a message trap in service-like application. That should be guaranteed to yield the control frequently and predictably.
- The function will always set the return value. The return value should be correct (as defined) for all accepted arguments. What will happen with corner-case input? Unacceptable arguments should be checked early and refused.
- The variables used should be able to hold any acceptable value without a loss of accuracy. For example monetary values should not be stored as floating point.
- Exceptions should never be used for expected. For example using an exception to leave a search loop when the item is found is sinful.
- All errors should be handled. Resources should be released even if an exception or an error condition occurred.
- Constants should be extracted, consolidated, properly named and placed at a predictable place (usually the top of the program unit, a configuration file or sometimes a specialized singleton).
- Are all possible null pointers checked before use?
- Could array indexes get out of bounds? Could a buffer overflow?
- Could SQL triggers be avoided?
- Are we using undocumented or deprecated features? Is that use properly commented? Are all workarounds commented?
- What 3rd party products we depend on? (How comes this was not handled at the architecture phase?)
- Do we need so many program units? (How comes this was not handled at the design phase?)
- Can recursion be avoided?
- Could SQL cursors be avoided?
- Is the memory (including heap, stack and disk) used sensibly? Objects should not be duplicated if not necessary. What about non-parallel threads? Are pooled resources retuned as soon as possible?
- Does the code alternate for an existing system/framework/library function without a proven need or advantage?
- Check all values coming from the user.
- Never execute user input. For example, do not use user input to build a sql statement on-the-fly.
- Encapsulation should be used wherever possible.
- Where and how are authentication data stored?
Feel free to leave any questions or comments below
Code Review continues in Part 5) Bugs found, now what?