phetsims / tandem

Simulation-side code for PhET-iO
MIT License
0 stars 5 forks source link

How to treat parametric types that have no parameters, such as EmitterIO<>? #216

Closed samreid closed 3 years ago

samreid commented 3 years ago

From https://github.com/phetsims/tandem/issues/211, there is a TODO in the IOType constructor:

    // Validate that parametric types look as expected
    // REVIEW: What about EmitterIO<> that has no parameter types?  See https://github.com/phetsims/tandem/issues/211
    // if ( this.typeName.includes( '<' ) ) {
    //   assert && assert( this.parameterTypes.length > 0,
    //     'angle bracket notation is only used for parametric IO Types that have parameter IO Types' );
    // }

Should EmitterIO that emits nothing be a parametric type (like other EmitterIO) or a non-parametric type (since it doesn't emit anything)? I'm uncertain of the assumptions or ramifications for studio. @zepumph can you please make a recommendation?

If necessary, we could change the default for parameterTypes to null (meaning non-parametric) instead of [] (meaning parametric with 0 parameters).

samreid commented 3 years ago

This seems higher than medium priority, and related to https://github.com/phetsims/tandem/issues/223. Marking "high" until we know more.

zepumph commented 3 years ago

I think this is slightly a duplicate of https://github.com/phetsims/tandem/issues/223. Personally, I don't feel like we need to do anything with this issue. It is more important to me that we make sure that parameterTypes are set correctly, and not that <> are exclusive only to a type with parameters, as it will never be a complete list (like for Function).

Feel free to close this issue if you agree.

samreid commented 3 years ago

I agree it is perfectly natural to use types like ActionIO<> etc. In the commit, I removed the commented-out code, closing.