Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicated instantiation of Guides #375

Closed
pixelzoom opened this issue Mar 15, 2022 · 1 comment
Closed

Duplicated instantiation of Guides #375

pixelzoom opened this issue Mar 15, 2022 · 1 comment
Assignees

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 15, 2022

For #374

Duplication in ArrowObjectScene and FramedObjectScene. The only thing that differs is the object arguments and phetioDocumentation.

    // Guides
    if ( optic instanceof Lens ) {
      this.guides1 = new Guides( this.optic, this.framedObject.positionProperty, {
        tandem: providedOptions.tandem.createTandem( 'guides1' ),
        phetioDocumentation: 'guides associated with the first point-of-interest on the framed object'
      } );
      this.guides2 = new Guides( this.optic, this.secondPoint.positionProperty, {
        tandem: providedOptions.tandem.createTandem( 'guides2' ),
        phetioDocumentation: 'guides associated with the second point-of-interest on the framed object'
      } );
    }
    else {
      this.guides1 = null;
      this.guides2 = null;
    }
@pixelzoom pixelzoom self-assigned this Mar 15, 2022
@pixelzoom pixelzoom changed the title Duplication in ArrowObjectScene and FramedObjectScene Duplicated instantiation of Guides Mar 15, 2022
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Mar 15, 2022

This turned into a bit of a mess -- doable, but at the expense of code readability. The optical objects are created by the subclass, and their positions are need to instantiate the guides. in the case of secondPoint, it's not really an optical object, just a position.

So at the expense of a few lines of duplicated code, I think it's best to keep things as is. The base class is responsible for creating things that are related to the optic. The subclasses are responsible for everything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant