Thursday, August 10, 2017

Code Quality Series - Use Simple Conditions to Increase Debuggability

Background


A practice commonly adopted by developers is to stuff multiple evaluation in a single assignment or evaluation.

e.g.
The use of functions in evaluations:


var oldAutoOrder = !order.isManual && helper.isOldOrder(order);

alternatively, something like this:


if (!order.isManual && helper.isOldOrder(order))

Property chaining:


if (time1.filledTime.DayName==="Sunday" || time1.filledTime.DayName==="Saturday")

I have used JavaScript code in examples but the problem stands for any programming language in general.

The Problem


The problem with this style of coding is that in case there are errors on this line of code and there is a need to debug the code, specially interactively, the current style would not allow proper debugging for following reasons:


  • Interactive debuggers normally do not show return values of the executed function in watch windows.
  • Debuggers that allow interactive evaluation of functions will run the function again on each evaluation.
  • Re-evaluation has a performance penalty
  • Functions are generally non-deterministic due to various factors which may not be under debugger's control, in which case the re-evaluation can result a different value compared to how the function actually failed.
  • Apart from a different return value, the re-evaluation may result in different code path being executed altogether.



The second problem, is with member (properties and functions) chaining, this deserves an article of it's own, but in the context of evaluations we just need to understand that when the conditions fail, object is null or undefined. Good luck figuring out where the problem has occurred. The above example is a very simplistic example and can arguably be deemed more efficient than the solution that I am going to propose. However, this code style where property names are not reported as part of the error will increase debugging complexity.

The Solution


The solution is very simple. Hold the values of functions in variables and build conditions using the variables instead of calling functions or chaining members.

e.g.


var isManualOrder = order.isManual;
var isOldOrder = helper.isOldOrder(order);

var isOldAutoOrder = !isManualOrder && isOldOrder;

if (isOldAutoOrder)


Similarly, with member chaining.

If there are no guarantees that a property will never be null or a that a function chain will always behave in a deterministic way, then chains should always be broken down into simple statements, so individual members can be evaluated at the time of debugging and this also allows proper validation of each member required to correctly come to the logical (read Boolean) answer.



No comments:

Post a Comment