Remove duplicated property from PoolWithFaucetsModel

[samreid]

During code review #88 @jbphet & I saw this code in PoolWithFaucetsModel.js

  function PoolWithFaucetsModel( underPressureModel ) {

    PropertySet.call( this, {
      volume: 1.5
    } );

    // Enable faucets and dropper based on amount of solution in the beaker.
    this.volumeProperty.link( function( volume ) {
      this.inputFaucet.enabled = ( volume < this.maxVolume );
      this.outputFaucet.enabled = ( volume > 0 );
      underPressureModel.currentVolume = volume;
    }.bind( this ) );
  }

The problem is that PoolWithFaucetsModel.volumeProperty and UnderPressureModel.currentVolumeProperty are duplicated. Can PoolWithFaucetsModel.volumeProperty be deleted and UnderPressureModel.currentVolumeProperty used instead?

[notsiddhartha]

PoolWithFaucetsModel.volumeProperty is required to maintain the volume in a particular scene.

UnderPressureModel.currentVolumeProperty is just a "simulated" derived property that depends on currentScene and the volume in the current scene. This property is passed to barometer so that it can react to changes in the volume. This way the BarometerNode doesn't need to know about the different scenes and can depend only on the UnderPressureModel's properties.

Is there a cleaner way to do this?

[samreid]

Instead of declaring a new volume Property in PoolWithFaucetsModel, why not use the currentVolumeProperty from the underPressureModel?

[notsiddhartha]

Each scene could have a different volume. If we use currentVolumeProperty from the underPressureModel then volume changes in one scene will reflect in the other.

[samreid]

That makes sense, can you please document that in the code?

[samreid]

Or, another way I could envision doing this (let me know what you think):

The currentVolumeProperty is a derived property based on the currentScene index and the values of the volume in each of the scenes--it would automatically update when the currentScene index or individual volumes change.

[notsiddhartha]

Will document the current approach in code.

Reg. the derived property approach, I think that will make UnderPressureModel depend on all other scene models and vice-versa. I am not aware of how to declare a derived property that depends on properties from different models. Could you point to an example?

[samreid]

I think that will make UnderPressureModel depend on all other scene models and vice-versa.

If this was a cyclic dependency, it wouldn't work. But I don't think a cyclic dependency is necessary here.

I am not aware of how to declare a derived property that depends on properties from different models.

It looks like addDerivedProperty does not support dependencies in other models at the moment. Perhaps that feature will be added in the future, but for now it seems best to document as you described above as:

Each scene could have a different volume. If we use currentVolumeProperty from the underPressureModel then volume changes in one scene will reflect in the other.

[notsiddhartha]

Documented above. Assigning to @samreid for review.

[samreid]

Perfect, thanks! Closing.