private field of PhetioGroup is used

[pixelzoom]

This sim has been repeatedly referenced as a good example of how to use PhetioGroup, including in the "How to Instrument..." doc. In 3 places, it's violating the PhetioGroup API and accessing private field memberDisposedEmitter. For example in ChargesAndFieldsScreenView:

      // Add the removal listener for if and when this electric field sensor is removed from the model.
      model.electricFieldSensorGroup.addMemberDisposedListener( function removalListener( removedElectricFieldSensor ) {
        if ( removedElectricFieldSensor === addedElectricFieldSensor ) {
          electricFieldSensorNodeGroup.disposeMember( electricFieldSensorNode );
          model.electricFieldSensorGroup.memberDisposedEmitter.removeListener( removalListener );
        }
      } );

Recommended to fix this immediately, so that this doesn't propagate to other sims. And I don't think the solution is to make memberDisposedEmitter, since the add*Listener methods are clearly intended to make the Emitters private.

[pixelzoom]

It might be OK to make PhetioGroup memberCreatedEmitter and memberDisposedEmitter public, then delete the add*Listener and remove*Listener methods. I don't feel like they're adding much in the way of encapsulation, and call sites might be more clearer, e.g.:

myGroup.addMemberCreatedListener( member => {...} );

myGroup.memberCreatedEmitter.addListener( member => {...} );

But I don't feel strongly either way. What I do feel strong about is that the Emitters and listener methods should not both be public, because that would result in 2 ways of doing things.

[zepumph]

I remember opinions about this from @chrisklus and @samreid when we created this. Assigning them.

[zepumph]

Oops! We have a convenience function for this, we just forgot to use it. Over to you @pixelzoom.

[zepumph]

Assigning back @samreid and @chrisklus in case they have opinions about wanting to make the emitters public. I don't care strongly, but prefer the way it is now until there is a time that we NEED access to the emitter.

[samreid]

I've been on both sides of this fence, but at the moment, it seems preferable to leave the emitter as private and use the methods. I don't feel strongly at all though.

[pixelzoom]

https://github.com/phetsims/charges-and-fields/commit/6ed747eaa4c830efb9dda631b23e8138c78f6e2e looks good.

[zepumph]

@chrisklus feel free to close if you agree, otherwise we can continue discussion.

[chrisklus]

Looks good to me, closing.