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

reuse of code for point tool #18

Closed
pixelzoom opened this issue Aug 21, 2018 · 5 comments
Closed

reuse of code for point tool #18

pixelzoom opened this issue Aug 21, 2018 · 5 comments
Assignees

Comments

@pixelzoom
Copy link
Contributor

Graphing Lines has a point tool that looks like this:
screenshot_717

Graphing Quadratics has a point tool that is similar, but has a different "crosshair" sensor:
screenshot_718

To implement the point tool in Graphing Quadratics, PointToolNode was copied from graphing-quadratics, then modified, with this TODO in the code:

//TODO Copied from GRAPHING_LINES/common/view/PointToolNode

The implementations are very similar, but factoring something out to be used by both graphing-lines and graphing-quadratics would be a bit forced/unnatural.

Options:
(1) Use the new design in both graphing-quadratics and graphing-lines.
(2) Factor out common code, even if it's forced/unnatural.
(3) Live with code duplication.

Option (1) is best, if that's acceptable to the designer. @amanda-phet what do you think?

@pixelzoom
Copy link
Contributor Author

Mentioning @ariel-phet, in case he has an opinion about this.

@pixelzoom pixelzoom mentioned this issue Aug 22, 2018
2 tasks
pixelzoom added a commit that referenced this issue Aug 22, 2018
Signed-off-by: Chris Malley <cmalley@pixelzoom.com>
@ariel-phet
Copy link

@pixelzoom @amanda-phet I would be fine with using the same style both, except for one "point". One tool goes to integer values, one is a bit more of "trace" tool. I don't think this is necessarily a massive or important distinction, and I think the UX will be fine either way.

@amanda-phet
Copy link
Contributor

The two tools have different behaviors, so the different styles make sense to me. The point tool in GL snaps to integer coordinates, and the point tool in GQ is more continuous. However, I am fine with option (1) if it is the best option from a development perspective.

@amanda-phet amanda-phet assigned pixelzoom and unassigned amanda-phet Aug 28, 2018
@pixelzoom
Copy link
Contributor Author

I'm starting to lean towards (3), live with duplication. Graphing Lines dealt with straight lines, and the point tool therein doesn't know how to deal with curved lines (like quadratics). And rather than try to make it deal with curved lines, I'm inclined to create a new point tool in Graphing Quadratics that handles both curved and straight lines.

@pixelzoom
Copy link
Contributor Author

I've decided not to reused PointToolNode. And in fact, I'm going to create a PointTool (model element) that is specific to Graphing Quadratics, because the one in Graphing Lines is (not surprisingly) specific to straight lines.

Closing.

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

3 participants