odd tandem for CuvetteNode

[pixelzoom]

In BeersLawView:

    var lightNode = new LightNode( ..., tandem.createTandem( 'light' ) );
    var cuvetteNode = new CuvetteNode( ..,. tandem.createTandem( 'cuvette' ).createTandem( 'node' ) );
   ...
    var detectorNode = new ATDetectorNode( ..., tandem.createTandem( 'ATDetectorNode' ) );
    var rulerNode = new BLLRulerNode( ..., tandem.createTandem( 'ruler' ) );

Why does CuvetteNode require a tandem that is further qualified with 'node'?

[samreid]

Here is the corresponding API declaration:

cuvette: {
  width: TProperty( TNumber ),// goes from 0.5 to 2
  node: TCuvetteNode
},

We could alternately change this to be something more like:

cuvette: TCuvetteNode.extend({
  width: TProperty( TNumber ),// goes from 0.5 to 2
}),

Or we could code the TProperty in the TCuvetteNode api like this:

api:{
  width: TProperty( TNumber )// goes from 0.5 to 2
}

These latter two options are described in Tips and Tricks in the "Extending wrapper types" section at the bottom of https://github.com/phetsims/together/blob/master/doc/how-to-instrument-a-phet-simulation.md

I wasn't sure which would be best in this case.

[pixelzoom]

So this is a workaround, and exposes a deficiency of the design. If I were to add a similar TProperty to Light, ATDetector or Ruler, then I would need to apply the same workaround to LightNode, ATDetector or RulerNode (respectively), and that would require a breaking change to beers-law-lab-api.js.

[pixelzoom]

Perhaps the general problem is that there's distinction between model and view in tandem creation. For example, in beers-law-lab-main.js we have:

new BeersLawScreen( tandem.createTandem( 'beersLawScreen' ) )

And BeersLawScreen does this:

Screen.call( 
      ...
      function() { return new BeersLawModel( ..., tandem ); },
      function( model ) { return new BeersLawView( ..., tandem ); }, {
        tandem: tandem
      }
    );

So the same tandem is used by BeersLawModel and BeersLawView, which opens up the door to namespace collision.

function() { return new BeersLawModel( ..., tandem.createTandem( 'model' ); },
function( model ) { return new BeersLawView( ..., tandem.createTandem( 'view' ); }

I realize that we're trying to simplify the API for the clients. But removing the model-view distinction seems to be throwing the proverbial baby out with the bathwater.

[pixelzoom]

Related to #99

[samreid]

I'm open to trying

function() { return new BeersLawModel( ..., tandem.createTandem( 'model' ); },
function( model ) { return new BeersLawView( ..., tandem.createTandem( 'view' ); }

or even

function() { return new BeersLawModel( ..., tandem.createTandem( 'beersLawModel' ); },
function( model ) { return new BeersLawView( ..., tandem.createTandem( 'beersLawView' ); }

Not sure what is best here, perhaps the former? @pixelzoom shall I try reinstrumenting beers-law-lab with this paradigm?

[pixelzoom]

@samreid and I had a Skype call to discuss #129 (comment).

With the current approach (aggregating model and view into a single tandem id), we'll be in trouble if we need to tandem.addInstance for both the model and view instances. And creating multiple tandems with the same id seems like a potential maintenance issue.

If we use separate tandem ids for model and view, then the API matches the code structure, which could make things easier to instrument and maintain. But by exposing the code structure, we make it more likely that we'll break the API if the code structure changes (or need to workaround breaking it in ways that compromise maintenance.)

[samreid]

I'm inclined to investigate separate tandems for model and view and live with the consequences of letting the sim implementation details leak into the API file. If particularly egregious API structures arise, it could be a hint for us to refactor the simulation implementation.

[pixelzoom]

If we standardize on separation of model and view tandems (see #140), then this looks good. I'll leave this open until that issue is closed.