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

NeuronSharedConstants doesn't need a constructor #34

Closed
jbphet opened this issue Dec 3, 2014 · 4 comments
Closed

NeuronSharedConstants doesn't need a constructor #34

jbphet opened this issue Dec 3, 2014 · 4 comments
Assignees

Comments

@jbphet
Copy link
Contributor

jbphet commented Dec 3, 2014

We generally us a pattern where the constants are just a static object, since there is no need to instantiate. See AreaBuilderSharedConstants.js for an example.

Found during code review, see #29.

@jbphet
Copy link
Contributor Author

jbphet commented Dec 3, 2014

In continuing with the review, it appears that there is a NeuronSharedConstants and a NeuronConstants type, and the latter adheres to the requested pattern. These should probably be consolidated.

@AshrafSharf
Copy link

NeuronSharedCOnstants and NeuronConstants are consolidated into one class NeuronConstants which adheres to Object.freeze pattern

@AshrafSharf AshrafSharf assigned jbphet and unassigned AshrafSharf Dec 8, 2014
@jbphet
Copy link
Contributor Author

jbphet commented Dec 15, 2014

Here is the commit for this one: 5363cb0.

@jbphet
Copy link
Contributor Author

jbphet commented Dec 15, 2014

Looks good, closing.

@jbphet jbphet closed this as completed Dec 15, 2014
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