-
Notifications
You must be signed in to change notification settings - Fork 83
testing exercises complete #28
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: master
Are you sure you want to change the base?
Conversation
| return false; | ||
| } | ||
|
|
||
| return arr1.concat(arr2).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.
one gotcha to be aware of with sort - it won't sort numbers correctly! The default sort in JavaScript is alphabetical, so for example:
[23,1,4].sort() // [1, 23, 4]
The example doesn't catch this.
To fix this, you can pass in a comparator to the sort function telling it how it should sort. reference
elie
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.
Great job! Set indentation to be 2 spaces in your editor please
| } | ||
| if (length > string.length) | ||
| return paddedStr.slice(0, length); | ||
| else |
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.
no need for an else here
tigarcia
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.
Good job. This game gets much harder with the exploding particles.
| }, | ||
| url: "https://hn-favorites.herokuapp.com/stories.json", | ||
| data: { | ||
| hacker_news_story: { ///// THIS IS FAILING. JSON Turns this into a bad string |
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.
Not sure what's going on here, but you're right that the ajax is failing. It's not sending any data in the http request.
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.
Oh, did you want:
favContainer[$key].$by
You should probably remove the $ from the property name.
| return $.get(`https://hacker-news.firebaseio.com/v0/item/${currentVal}.json`) | ||
| }); | ||
|
|
||
| return Promise.all(promises) |
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.
Nice job using Promise.all
| @@ -1,0 +1,107 @@ | |||
| // - Write a function called `sumEvenArguments` which takes all of the arguments passed to a function and returns the sum of the even ones. | |||
| function sumEvenArguments() { | |||
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.
Everything looks good for these exercises.
| console.log(p.fullName()); // Shana Malarkin | ||
|
|
||
| Person.prototype.toString = function() { | ||
| return this.firstName + " " + this.lastName + ", Favorite Color: " + this.favoriteColor + |
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.
You could also use this.fullName() here.
|
|
||
| Person.prototype.addToFamily = function(object) { | ||
| if (object instanceof Person) | ||
| if (this.family.indexOf(object) === -1) |
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.
It's a matter of preference but I prefer curly braces around these if statements. That way you can't accidentally forget the curly if you add more lines later.
| } | ||
|
|
||
| Array.prototype.map = function(fn, thisArg) { | ||
| // if (thisArg) this = thisArg; How do we handle this?! |
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.
good question, you should probably check if the argument was provided, and if it was, use it like so:
newArray.push(fn.call(thisArg, this[i], i, this));
| */ | ||
|
|
||
| Array.prototype.reduce = function(fn, startVal) { | ||
| var startValue = startVal? startVal : this[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.
This actually doesn't work because our starting value could be 0 or "" which is falsy. The best way to do this is to check the length of the arguments array.
@mmmaaatttttt Exercises for Testing