meanProperty should be a DerivedProperty

[pixelzoom]

For code review #41 ...

For this checklist item:

• Are all dependent Properties modeled as DerivedProperty instead of Property?

This is unfortunate in IntroModel.ts, especially since it's arguable the most-important Property in the screen:

    // This value is derived from the water levels in all the cups, but cannot be modeled as a DerivedProperty since
    // the number of cups varies
    this.meanProperty = new NumberProperty( ...

And this unfortunate compromise is due to #60. So to reiterate what I said in that issue, I would reconsider the decision to dynamically allocate pipes and cups. Or investigate other ways of allowing meanProperty to be a DerivedProperty.

[samreid]

We also used the new feature deriveAny from phetsims/axon#406.

[samreid]

@marlitas and I collaborated on this, and it seems ready for review. Thanks for the great recommendation!

[pixelzoom]

Looks good. A couple of nits...

• This comment means nothing to me. Maybe say it differently or more verbosely. Is it saying something about the internal implementatioin of DerivedProperty? If so, what if that changes?

    // map() does not preserve a property of .length required for DerivedProperty
    this.meanProperty = DerivedProperty.deriveAny( dependencies,

• Since it's public-facing... The phetioDocumentation for meanProperty is very difficult to read/parse, because it contains compound adjectives that are not properly hyphenated. (If you're not familiar with compound adjectives and when to hypenate, see https://www.gingersoftware.com/content/grammar-rules/adjectives/compound-adjectives/.) Recommended change:

-         phetioDocumentation: 'The ground truth water level mean.',
+         phetioDocumentation: 'The ground-truth water-level mean.',

[pixelzoom]

Maybe the comment is trying to explain why dependencies is an array, and not a map? If so, the comment probably belongs above the definition of const dependencies. But even then, you still need to use the spread operator (...) to combine the maps into one array. So maybe the comment is unnecessary?

    const dependencies = [
      ...this.waterCup3DArray.map( waterCup => waterCup.waterLevelProperty ),
      ...this.waterCup3DArray.map( waterCup => waterCup.isActiveProperty )
    ];

    // map() does not preserve a property of .length required for DerivedProperty
    this.meanProperty = DerivedProperty.deriveAny( dependencies,

[marlitas]

Documentation updated in above commit. Hopefully this clarifies the comment before meanProperty. I do not fully understand the typing restrictions on derivedProperty, I just know they exist, so if further clarification is needed I'll reach out for assistance.

Assigning back to @pixelzoom for review.

[pixelzoom]

👍🏼 closing.