Calls to optionize that do nothing

[pixelzoom]

For code review #41 ...

In IntroModel.ts:

const options = optionize<LevelingOutModelOptions, SelfOptions, MeanShareAndBalanceModelOptions>()( {}, providedOptions );

This same pattern is used in MeanShareAndBalanceScreenView.ts and IntroScreenView.ts. And you seem to have used this pattern consistently.

It's nice that providedOptions only appears in optionize call, and options is then used throughout the constructor. That makes it consistent with other constructors. But I'm not sure if it's worth the additional call to optionize.

No need to change anything, but I would be interested in hearing your motivation for using this pattern.

[marlitas]

I will say that my main reason is consistency. Coming in and building this out it was nice to have that pattern at the top of every constructor. It was one less thing to have to keep track of, as well as allowed for easy scaling if a class did need options down the line. However, I see how this is not necessary and will refactor to eliminate in classes that are not using options at all.

[marlitas]

Sending back to @pixelzoom for re-review. Do these changes feel more appropriate to PhET patterns and conventions?

[pixelzoom]

Thanks for explaining the motivation.

There doesn't really seem to be a PhET pattern/convention for this (should there be?), so I can't evaluate your changes in that context. And like I said initially, I can see value (consistency) in the way that you originally did this. Your solution in c402c5a seems nice, so that options is used thoughout the constructor.

If you're happy with the changes, go ahead and close. If you're still unsure, maybe solicit the opinion of @samreid or one of the other developers about whether no-op optionize calls are OK.

[marlitas]

These changes work for me, and reduce unnecessary work with an empty optionize call. Thanks for the feedback @pixelzoom! Closing.