Software has two ingredients: opinions and logic (=programming). The second ingredient is rare and is typically replaced by the first.
I blog about code correctness, maintainability, testability, and functional programming.
This blog does not represent views or opinions of my employer.

Monday, April 23, 2012

Imperative curlies 6: removing ifs

In last few posts I tried to argue that there is little or no code reuse around for loops.  There is one notable exception, however, the code reuse by adding lots of additional curlies called if statements.   This post contains my thoughts on how to change imperative ifs used in this way.

Often the developer is faced with two obvious options, repeat very similar (but not identical) imperative logic in several places or code the logic in one place and include lots of if statements within this ‘generalized’ logic to handle the differences.  The generalized logic is often a private implementation method with several Boolean parameters.  It is invoked from various public methods which will set the booleans to trigger the needed side-effects within the private method.  Obviously many programming techniques have been developed to avoid such code (Template Method Design Pattern, or even the concept of polymorphism itself),  but still the if statements are often easier to use.

Here is a piece of code taken directly from the open source Ext JS 4.0.7 (fragment of Ext.form.Basic):
01: getValues: function(asString, dirtyOnly, includeEmptyText, useDataValues) {
02:  var values = {};
04:   this.getFields().each(function(field) {
05:      if (!dirtyOnly || field.isDirty()) {
06:         var data = field[useDataValues?'getModelData':'getSubmitData'](includeEmptyText);
07:         if (Ext.isObject(data)) {
08:             Ext.iterate(data, function(name, val) {
09:                 if (includeEmptyText && val === '') {
10:                     val = field.emptyText || '';
11:                 }
12:                 if (name in values) {
13:                     var bucket = values[name],
14:                         isArray = Ext.isArray;
15:                     if (!isArray(bucket)) {
16:                         bucket = values[name] = [bucket];
18:                     }
19:                     if (isArray(val)) {
20:                         values[name] = bucket.concat(val);
21:                     } else {
22:                         bucket.push(val);
23:                     }
24:                 } else {
25:                     values[name] = val;
26:                 }
27:             });
28:         }
29:     }
30:   });
32:   if (asString) {
33:      values = Ext.Object.toQueryString(values); 
34:   }
35:   return values;
36: }      
Ext.form.Basic is an Ext class defining base/reusable behavior of Ext Forms.  Ext JS uses the above method internally:
01: getFieldValues: function(dirtyOnly) {
02:    return this.getValues(false, dirtyOnly, false, true);
03: },      
Also,  the last argument (useDataValues)  is not documented, making getValues behave as an old style form submit value retrieval.   Instead of repeating the same logic and providing separate implementations to retrieve only dirty data from the form vs. all the data,  retrieve data using new model semantics vs. using old form submit,  etc, etc,  the Ext library codes the logic once and provides if conditionals within the logic to handle different cases.

Looking at the above snapshot, can you easily see what the code is doing?

The idea or re-implementing the code by the removing the curlies/shortening distance between curlies is somewhat unfair to the reimplementer.  After all the method signature has all these Booleans (or JavaScript truthies),  and I think the API designers had an imperative implementation in mind when defining the signature of this method.  Declarative/functional thinking vs imperative thinking will impact the APIs.

Lets do it anyway!   I am dropping the asString parameter and I will type it as a separate function.  I personally dislike function result set type changing based on the parameters.   Standard function composition can be defined this way:
f: X -> Y
g: Y -> Z
g Compose f: X -> Z

(g Compose f(x)) = g(f(x))
The more I code with JavaScript the more I find that it is more convenient to do this type of composition (compose with function which is aware of the original argument set):
f: X -> Y
g: Y, X ->Z
g ComposePlus f: X -> Z

(g ComposePlus f) (x) = g(f(x), x)
Assume that we have bunch of utilities coded for things like flexible array concatenation, and that the ‘getFields()’ method returns a rich JavaScript collection object loaded with nice methods like map, fold, filter.  My imaginary filterIf(condition, fn) method will return original collection if condition is not met and filter otherwise.  So here is the new version of the code:
01: getValues: function(dirtyOnly, includeEmptyText, useDataValues) {
02:   var dirtyOnlyAdjustF = function(field) { return field.isDirty(); };
04:   var fieldRetrievalF = useDataValues ? 
05:                           function(field) {return field.getModelData(includeEmptyText);} :
06:                           function(field) {return field.getSubmitData(includeEmptyText);};
09:   var emptyFieldAdjustF = function(data, field) {
10:      Ext.iterate(data, function(name, val) { 
11:         data[name] = (val === '') ? field.emptyText : '';
12:  }                                                                          
13:      return data;
14:   };
16:   if(includeEmptyText) {
17:      fieldRetrievalF = FunctionUtil.composePlus(emptyFieldAdjustF, fieldRetrievalF);
18:   }
20:   var foldingF = function(data, aggregator) {
21:      Ext.iterate(data, function(name, val) {
22:        var bucket = aggregator[name];
23:        aggregator[name] = bucket ? ArrayUtil.concatenate(bucket, val) : val;  
24:     }
25:   };
27:   var values = this.getFields()
28:                      .filterIf(dirtyOnly, dirtyOnlyAdjustF)
29:                        .map(fieldRetrievalF)
30:                          .fold({}, foldingF);   
31: },
33: getValuesAsString: FunctionUtil.compose(Ext.Object.toQueryString, this.getValues);
Lots of developers will say that the second code is better because it is fluent.  I think the fluency is more of a side-effect resulting from changing from imperative to declarative programming style.

Looking at the above code,  how easy is to figure out what the code is doing?   Actually the code says what it does, if statements never do that.

Many developers do not like the ternary operator.  I have used it here to emphasize the declarative aspect.   If the distance between the curlies is short, then we are close to being declarative.  I like when the language lets me drop the curlies to emphasize that the code has been sufficiently refactored from the imperative.   In this example JavaScript ternary operator does just that!

I hope the code speaks for itself so I will not say anything more about it.

1 comment: