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

missing visibility annotations for fields #190

Closed
pixelzoom opened this issue Nov 9, 2015 · 2 comments
Closed

missing visibility annotations for fields #190

pixelzoom opened this issue Nov 9, 2015 · 2 comments
Assignees

Comments

@pixelzoom
Copy link
Contributor

Related to code review #173.

Lots of examples... BodyConfig, ModeConfig, ModeList, ModeListParameterList ... Looks like someone should go through all of the code again.

All publicly-visible fields (e.g., this.foo) must have an associated visibility modifier. Assuming @public in the absence of an annotation is no longer the convention.

@aaronsamuel137
Copy link
Contributor

Added annotations. For some classes I commented like so:

    // all fields are @public
    this.fixed = false; // True if the object doesn't move when the clock ticks
    this.mass = mass;
    this.radius = radius;
    this.x = x;
    this.y = y;
    this.vx = vx;
    this.vy = vy;

Assigning to @pixelzoom for review.

@pixelzoom
Copy link
Contributor Author

@aaronsamuel137 If you have multiple fields that are public, and there are no blank lines between their declarations, the convention is to do this:

   // @public
    this.fixed = false; // True if the object doesn't move when the clock ticks
    this.mass = mass;
    this.radius = radius;
    this.x = x;
    this.y = y;
    this.vx = vx;
    this.vy = vy;

Similarly for a PropertySet, either of these is OK:

new PropertySet( {

  // @public
  mass: 10,
  radius: 10
} );

// @public
new PropertySet( {
  mass: 10,
  radius: 10
} );

For a PropertySet with Properties of varying visibility, you can do either of these:

// Example: annotate each property
new PropertySet( {
  p1: ...   // @public
  p2: ...   // @private
  p3: ...   // @public
  p4: ...   // @private
} );

// Example: group properties by visibility
new PropertySet( {

  // @public
  p1: ...
  p3: ...

  // @private
  p2: ...
  p4: ...
} );

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