-
Notifications
You must be signed in to change notification settings - Fork 0
#tight-wallaroo 176: Core Vanilla Javascript [Team Drills & Practice] #6
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
base: empty1
Are you sure you want to change the base?
Conversation
| expect(handshake.commands()).toEqual(['wink']); | ||
| }); | ||
|
|
||
| xit('10 is a double blink', function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when I looked it up, it appears that the xit() function skips the test? I assume you were working on this when the week ended :) Your code is looking good! I'd recommend putting up the code into github without the xit() even if the tests are failing.
| expect(result).toEqual('Whatever.'); | ||
| }); | ||
|
|
||
| xit('shouting', function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cloned your project so I could test it and give detailed feedback. It's looking great, with clean code, though it's difficult for me to test since many of the tests have been skipped.
| var Hamming = function() {}; | ||
|
|
||
| Hamming.prototype.checkForSameLength = function(firstStrand, secondStrand) { | ||
| return firstStrand.length === secondStrand.length ? true : false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a great use of the ? operator! Clean code, succinctly written.
| Hamming.prototype.compute = function(firstStrand, secondStrand) { | ||
| if (this.checkForSameLength(firstStrand, secondStrand)) { | ||
| return firstStrand.split('').reduce( function(accumulator, current, index){ | ||
| return current !== secondStrand[index] ? accumulator + 1 : accumulator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another place of terse well written code that's easy to understand. Nice!
| @@ -7,31 +7,31 @@ describe('Hamming', function () { | |||
| expect(hamming.compute('A', 'A')).toEqual(0); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsure of the @@ code before this expect function in your test. Perhaps the describe() was supposed to be on the next line?
| 'Ileana', 'Joseph', 'Kincaid', 'Larry' | ||
| ]) { | ||
| this.diagram = diagram.split('\n') | ||
| this.students = students.sort() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This must be because I'm new or something, but what kind of sort() is happening here? I feel a comment might be helpful :)
| @@ -0,0 +1,30 @@ | |||
| var Series = function(inputNumber) { | |||
| if( /[^0-9]/g.test(inputNumber) ) { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've noticed this nice errors catchers in all the right places in the code. Great! Thanks for teaching me this.
| return largestProduct | ||
| }; | ||
|
|
||
| module.exports = Series; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nicely coded module!
| if (givenNth < 1) { | ||
| throw new Error('Prime is not possible') | ||
| } | ||
| //count up towards infinity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great comment. Shows me your thoughtflow, which I feel is very useful.
bairbanu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean code, easy to read and follow, comments placed where they're most needed, was fun to review. Suggestion for future would be to enable tests so reviewer can run them and make more detailed suggestions :)
No description provided.