A better way to pass through configuration

[samreid]

While working on adding nested options to NumberControl, @pixelzoom said:

I still think there must be a better approach (than options) to customizing UI components.

I want to take a look at an idea I had for this.

[samreid]

The main idea: each object knows about its immediate children and provides defaults for them. If you want to provide customized structure for nested object, the client creates it. This eliminates the need for creating and mapping a multitude of variable names. For example:

function House( options ) {
  options = _.extend( {
    frontDoor: new FrontDoor(), // default front door
    roof: new Roof(), // default roof
    floor: new Floor() // default floor
  }, options );
}
function Roof( color ) {
  this.color = color || 'gray';
}
function Floor() {}
function FrontDoor() {}
function Doorknob( clockwise ) {
  this.clockwise = clockwise;
}

var myHouse = new House( {
  roof: new Roof( 'blue' ), // roof is default except for the color
  frontDoor: new FrontDoor( {  // front door is default except for the doorknob
    doorknob: new Doorknob( true ) // doorknob is default except for the clockwise
  } )
  // floor is default
} );

[samreid]

In the above example, the House constructor doesn't need to know anything about the substructure of Door (for instance, that it has a doorknob). If the client wants defaults, it does nothing. If it wants to override something, it has full control over how the overriding is done.

Con: in the worst case scenario, the scene graph gets constructed twice, once for options and once with its replacement. But I bet we can come up with a pattern that obviates this.

[samreid]

Here's a mock-up of our existing pattern, that we don't like too much:

function House( options ) {
  options = _.extend( {
    frontDoorDoorknobClockwise: true,
    roofColor: 'gray',
    floorTexture: 'carpet',
  }, options );
  this.frontDoor = new FrontDoor({doorknob: new Doorknob(options.frontDoorDoorknobClockwise)})
  this.roof = new Roof(options.roofColor);
  //etc.
}
function Roof( color ) {
  this.color = color || 'gray';
}
function Floor() {}
function FrontDoor() {}
function Doorknob( clockwise ) {
  this.clockwise = clockwise;
}

var myHouse = new House( {
  frontDoorDoorknobClockwise: false,
  roofColor: 'blue'
  // floor is default
} );

[samreid]

Some thoughts about NumberControl:

  function NumberControl2( options ) {

    // Don't double create everything, nice!
    var titleNode = options.titleNode || new Node(); // Any kind of node can be used for the title, nice!
    var slider = options.slider || new HSlider();
    var leftArrowButton = options.leftArrowButton || new ArrowButton( 'left', function() {}, {} );

    // Con: Some things are likely duplicated with how client would create them.
    var rightArrowButton = options.rightArrowButton || new ArrowButton( 'right', function() {}, {} );

    // For instance, the same property should be specified in HSlider and NumberDisplay.  Can we "curry" it out, or is
    // it OK that it must appear multiple places?

    // Or what if you can do Component.setProperty() and it would use the new property?  That would open up flexibility for this pattern.
    var numberDisplay = options.numberDisplay || new NumberDisplay();

    Node.call( this, { children: [ titleNode, slider, leftArrowButton, rightArrowButton, numberDisplay ] } );
  }

How to make it easy for all subcomponents to use the same Property? Perhaps add setProperty to them, or let them have their own properties, and bind them using a two-way binding.

EDIT: Or curry it out, by providing a function that takes a Property and returns a Component. Then instead of passing in a Component instance, the client would pass in a function that takes everything except the Property, and the component supplies the Property.

[samreid]

I fleshed out a NumberControl2 prototype and example usage. This pattern seems promising, some thoughts:

1. Composite components such as NumberControl should not have to know all about subcomponents structure and options, and sub-subcomponents structure and options, etc. It should only know about its direct descendants and its own parameters. This pattern captures that pattern nicely.
2. This pattern avoids extraneous allocations with _.extend(), by only creating things when they are needed.
3. This pattern is much more flexible. For instance, I replaced an ArrowButton with a TextPushButton in NumberControl2 and everything worked perfectly. This did not required adding extra options or passing them through.
4. I was worried about having to pass the same property to multiple places, but in practice this seemed acceptable, and again opens up flexibility.
5. Even if this is a great pattern, it would require a herculean effort to port pre-existing components to use this pattern because it entails a significant API change. That being said, I'd be interested to see what a full stack of components and usage sites would look like if we pursued this pattern. And, it should be considered for future development.

[samreid]

I'd like to discuss this pattern with @pixelzoom before having a larger group discussion. @pixelzoom let me know your thoughts at your convenience, or if you would like to voice chat about it. The changes above are committed to scenery-phet/configuration branch and you can test it by running http://localhost/scenery-phet/scenery-phet_en.html?screens=2

[samreid]

The example usage is given in SlidersView.js:

    // NumberControl with default layout
    var numberControl1 = new NumberControl2( 'Weight:', weightProperty, weightRange, numberControlOptions );

    // A customized one
    var numberControl2 = new NumberControl2( null, weightProperty, weightRange, {
      titleNode: new Text( 'HELLO THERE', {
        fontSize: 20
      } ),
      slider: new HSlider( weightProperty, weightRange, { thumbFillEnabled: 'green' } ),
      leftArrowButton: new TextPushButton( 'REDUCE', {
        listener: function() {
          weightProperty.value = Math.max( weightProperty.value - 10, weightRange.min );
        }
      } )
    } );

And I haven't made any effort to implement various layouts. It should look like this:

[pixelzoom]

My quick evaluation...

Pros:

1. Component may have a smaller default options hash
2. Component doesn't need to propagate/forward option values to its subcomponent
3. Coarser grained customization: ability to use different types of subcomponents, as long as they implement a specified interface.

Cons:

1. Weak/leaky encapsulation: Exposes the inner workings of the component - how many subcomponents it has, the (duck?) type of those subcomponents, etc.
2. Substantial increase in the documentation required to describe the (duck?) type of subcomponents. We'll be relying more heavily on interfaces in a language (JS) that doesn't support them.
3. Substantial increase in validation code required to verify that subcomponents provided by clients comply with required interfaces.
4. Increased need for consistency of names across things that might be used as subcomponents. (Consistency is good, but now essential if relying duck typing.)
5. If a client wants to change even 1 option default (which happens frequently and it the whole point of providing options) then the client will need to create its own subcomponent(s). That's a substantially heavier responsibility for the client, and has the effect of locating more implementation details at the client call site.
6. If "coarse grain customization" is used (using different subcomponent types), it may become difficult to identify the concrete types that are used as subcomponents for a component.
7. Cons 1,5,6 will make it substantially more difficult to change the implementation of a component, since they leak implementation details to client sites.

[samreid]

Are we facing the problem of dependency injection?

[samreid]

Yes, it appears this is the same problem as dependency injection, and the standard solution for this problem is an inversion of control container. The simplest form of this is to provide a service or function that, when called, returns an appropriate value. I'm trying this for NumberControl and it seems promising.

[samreid]

I'm testing out:

      createTitleNode: function( title, options ) {
        return new TandemText( title, options );
      },

But we could alternatively use:

      TitleType: TandemText

I'm experimenting with the former first because it has direct lookups instead of dynamic names.