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

Replace x and y with position #77

Closed
pixelzoom opened this issue Jun 28, 2022 · 1 comment
Closed

Replace x and y with position #77

pixelzoom opened this issue Jun 28, 2022 · 1 comment
Assignees

Comments

@pixelzoom
Copy link
Contributor

For code review #41 ...

PipeModel and WaterCupModel (and their IOTypes) have options and fields named x and y. For example in PipeMode.ts:

type StateObject = {
  x: number;
  y: number;
}

type SelfOptions = {
  //REVIEW non-obvious options are supposed to be documented where defined, and these are not obvious to the reviewer
  //REVIEW if x and y are the pipe's position, then position: Vector2 is preferable
  x: number;
  y: number;
  isOpen?: boolean;
};
...
  public readonly x: number;
  public readonly y: number;
``` 

While `x` and `y` are undocumented, I gather that they are the position of the model element.  In that case `position: Vector2` is generally preferred. 

Also note also from the CRC:

> - [ ] PhET prefers to use the term "position" to refer to the physical (x,y) position of objects. This applies to both brands, but is more important for the PhET-iO API. See https://github.com/phetsims/phet-info/issues/126
@marlitas
Copy link
Contributor

x & y replaced with position. 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

2 participants