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

Include preferred coding patterns somewhere in the documentation #232

Open
AgustinVallejo opened this issue Feb 20, 2025 · 1 comment
Open

Comments

@AgustinVallejo
Copy link
Contributor

As part of phetsims/models-of-the-hydrogen-atom#125 I was discussing with @pixelzoom about the following conventions for exposing fields:

  public readonly positionProperty: TReadOnlyProperty<Vector2>;
  private readonly _positionProperty: Property<Vector2>;

To avoid clients or outside classes from modifying private fields. I realized this is not a pattern I really use in my sims and was wondering if it'd be worth promoting into our convention documentation, as others might be unaware of these details.

@pixelzoom
Copy link
Contributor

pixelzoom commented Feb 20, 2025

This pattern concerns the situation where a field (often a Property) is read-only for the public API, but writeable internally to the class. Two common implementation patterns are shown below. @AgustinVallejo was commenting on the first pattern, which is used in MOTHA. The anti-pattern shown below is unfortunately what I see too often in code reviews.

// Pattern 1: This pattern uses 2 references to the same Property instance. The public reference 
// is read-only for getting the value. The private reference is for setting and resetting the instance
// internal to the class.
class MyClass {

  public readonly positionProperty: TReadOnlyProperty<Vector2>;
  private readonly _positionProperty: Property<Vector2>;

  public constructor() {
     this. _positionProperty = new Vector2Property( ... );
     this. positionProperty = this.positionProperty;
  }

  public reset(): void {
    this._positionProperty.reset();
  }
}
// Pattern 2: This pattern uses 1 private reference to the Property instance and 
// a public ES5 getter.
class MyClass {

  private readonly positionProperty: Property<Vector2>;

  public constructor() {
     this. positionProperty = new Vector2Property( ... );
  }

  public reset(): void {
    this.positionProperty.reset();
  }

  get position(): Vector2 {
    return this.positionProperty.value;
  }
}
// Anti-pattern:  A single reference that is writeable in the public API, with documentation 
// saying "don't write to this", or an implicit hope that no one will write to it.
class MyClass {

  public readonly positionProperty: Property<Vector2>;

  public constructor() {
     this. positionProperty = new Vector2Property( ... );
  }

  public reset(): void {
    this.positionProperty.reset();
  }
}

@pixelzoom pixelzoom removed their assignment Feb 20, 2025
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