The Very Best Format for Dart Optional Constructor Parameters

‹prev | My Chain | next›

I dearly love optional parameters in Dart methods, functions and especially constructors. That said…

A long list of constructor arguments tends to raise my code-too-wide hackles. Consider the constructor for the Editor class in the ICE Code Editor. It has one required argument, a DOM element to hold the editor, and four optional, named parameters:

class Editor {
  Editor(this._el, {this.edit_only:false, this.autoupdate:true, this.title, this.enable_javascript_mode:true}) {
    this._startAce();
    this._applyStyles();
  }
  // ...
}

I like to keep my lines of code under 60 characters wide. This is partially superstition and partially legitimate suspicion. The suspicion is that wide code is hiding complexity, which is a natural enemy of maintainability.

It is hard to get too worked up about constructor arguments, but they do have a significant effect on behavior. The above constructor accepts an edit_only parameter, which defaults to false. It accepts autoupdate, which defaults to true. It accepts title, which is null by default. Finally it accepts enable_javascript_mode, which defaults to true.

OK, so maybe it is easy to get worked up about it when I list those out. Each of those have behavior implications and burying defaults in a long line of unrelated defaults seems like a good way to make the code and intent less readable.

I am not a fan of placing one parameter per line as it then becomes difficult to distinguish between constructor arguments and constructor body:

class Editor {
  Editor(this._el, {
    this.edit_only:false,
    this.autoupdate:true,
    this.title,
    this.enable_javascript_mode:true
  }) {
    this._startAce();
    this._applyStyles();
  }
  // ...
}

In my day dreams, it might be nice to use Dart's redirecting constructors to set optional parameters and then redirect to a “real” constructor:

class Editor {
  Editor.config(this._el, {
    this.edit_only:false,
    this.autoupdate:true,
    this.title,
    this.enable_javascript_mode:true
  }) : this();

  Editor() {
    this._startAce();
    this._applyStyles();
  }
  // ...
}

But that will not work because redirecting constructors cannot use field initializers like that.

So I am left to decide how best to format my largish constructor for optimum readability. It should be obvious that the list of parameters and constructor body are logically separate. It should be obvious that optional parameters are of less importance than required parameters. Perhaps something like this:

class Editor {
  Editor(
    this._el, {
      this.edit_only:false,
      this.autoupdate:true,
      this.title,
      this.enable_javascript_mode:true
    }) 
  { this._start(); }
  // ...
}

I think I can live with that. The constructor name, Editor, has the same indent level as the constructor body (which I squash into a single line). The required parameter, which is assigned to the _el private variable via field initialization, is clearly part of the constructor parameters. By indenting it, it makes the body stand out as logically different that parameters. Finally, by indenting the optional, named parameters, it further aids to distinguish between constructor parameters and body while also suggesting that the optional parameters are less important than the required parameter.

I think I can live with that. I may even like it. Except…

Now that I have the parameters unrolled like that, I finally ask myself if they are needed at all. In fact, the title parameter is never used in the Editor, so I eliminate that right away. The edit_only and autoupdate parameters are not used in any of the code or tests written so far, so neither needs to be in the parameter list (though their default values still need to be set). This leaves me with:

class Editor {
  Editor(this._el, {this.enable_javascript_mode:true}) {
    this._startAce();
    this._applyStyles();
    this._edit_only = false;
    this.autoupdate = true;
  }
  // ...
}

Actually, there are better ways to set default values for instance variables than setting them in constructors. Simply declaring them with default values will work fine:

class Editor {
  bool autoupdate = true;
  bool _edit_only = false;

  Editor(this._el, {this.enable_javascript_mode:true}) {
    this._startAce();
    this._applyStyles();
  }
  // ...
}

That is a lot better. The only thing that really stand out now is the enable_javascript_mode instance variable. In addition to visually standing out, it also bothers me that the only reason this instance variable exists is to support testing (it disables a web worker, which will not work with a file:// URL like test pages). In the setup of many of my tests in ICE, I use this to disable JavaScript mode:

    test("defaults to auto-update the preview", () {
      var it = new Editor('#ice-${currentTestCase.id}', enable_javascript_mode: false);
      expect(it.autoupdate, equals(true));
      expect(it.editorReady, completes);
    });

Now that I think about it—now that re-organizing my code forced me to think about it—that does not need to be an instance variable. Instead, I can make it a static variable:

class Editor {
  bool autoupdate = true;
  bool _edit_only = false;

  static bool disableJavaScriptWorker = false;

  Editor(this._el) {
    this._startAce();
    this._applyStyles();
  }
  // ...
}

Then, I can remove all of those enable_javascript_mode constructor parameters and instead set that static variable once:

library ice_test;

main(){
  Editor.disableJavaScriptWorker = true;
  // ...
}

So in the end, I think that I have a decent format for a large number of constructor arguments. But I think that I have an even better constructor without any optional parameters at all—ironically because I worked so hard to organize the same parameters in the first place. Regardless, the code is in better, more readable, maintainable shape, which is what is really important.


Day #797