Part1 - constructors and prototypes #5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 9 commits
December 9, 2016 22:54
- follow directions more closely
sheriffderek
changed the title
sheriffderek
changed the title
joshdmiller
reviewed
Dec 15, 2016
Owner
joshdmiller
left a comment
joshdmiller
left a comment
There was a problem hiding this comment.
The only thing I'd comment here is that this includes features not in the original test and that have no tests of their own. E.g. passing a parameter into the various speak functions.
src/index.js
Outdated
| // - I didn't understand what this was at first, but it's in the tests... | ||
| // - so, I have to give each constructor a 'constant' I guess... | ||
| // - "On the contrary... It is in uppercase because that's a common convention for constants defined on the constructor rather than on the prototype." | ||
| // - I still don't see the helpfulness of the all caps but I'm sure it's with reason to avoid some confusion |
Owner
There was a problem hiding this comment.
The all caps is pure convention, probably a hold-over from an older language. If you look at languages like Java and C, constants are defined in all caps by convention. Even though these are not technically constants (JavaScript has no such feature), we list them in all caps to signify our intent.
Author
|
So, this is without the added parameter being passed to speak. Seem good? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I wrote this over again to see If I could remember. How does it look?