-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A better way to pass through configuration #730
Comments
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
} ); |
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. |
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
} ); |
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 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. |
I fleshed out a NumberControl2 prototype and example usage. This pattern seems promising, some thoughts:
|
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 |
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: |
My quick evaluation... Pros:
Cons:
|
Are we facing the problem of dependency injection? |
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. |
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. |
While working on adding nested options to NumberControl, @pixelzoom said:
I want to take a look at an idea I had for this.
The text was updated successfully, but these errors were encountered: