Skip to content

Conversation

@GlebBabahov
Copy link

No description provided.

```

### Enumerations
- Enumeration classes are in

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend adding a line about how to name the class itself. This only covers the enumerated values.

- Settings that are essential to your code's functionality (like the number of degrees to turn for `GyroTurn`) can also be passed in

### Setting Properties
- If you need to override a default setting for a class, use the flow model

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by the flow model? I can infer from what it's doing, but I'm not sure this is covered for new developers. Focus on the pattern itself (flow pattern is when the setter method returns this.)

```

### Constructors
- If your class needs to use another classes' object, it should be passed into the constructor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of saying that you'll pass the specific classes, refer to them as dependencies. Also if the class implements an interface, the interface should be passed in to decouple from specific implementations.

Copy link

@oconnelc oconnelc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some cleanup needed but otherwise looks good.

@@ -0,0 +1,48 @@
### Variables
- Constants
- Preceded by `k`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a line about constants being static final members. This is because the fields says "Fields are any variables found in the class body." A new user may get confused as to which naming convention to use.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants