-
Notifications
You must be signed in to change notification settings - Fork 83
intermediate javascript exercise sols #29
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
|
|
||
| describe("TESTS FOR mergeObjects", function() { | ||
| it("two objects", function() { | ||
| var obj1 = {name: "Foo", num: 3}; |
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.
careful with indentation here
testing_exercise/testing.js
Outdated
| @@ -1,0 +1,48 @@ | |||
| function replaceWith(entry, replaceChar, withChar) { | |||
| if (entry.indexOf(replaceChar) !== -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.
nice! small thing - do you need this outer if statement?
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.
agreed, have taken out the outer if statement.
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 start! Try to finish the lodash exercises and set your indentation to be 2 spaces in your editor
lodash_exercise/lodash.js
Outdated
| if (num < 0) | ||
| return num > end && num < start; | ||
| else | ||
| return num < end && num > start; |
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.
Don't need an else here
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.
- indentation changed to 2 spaces on my editor.
- I tried taking off the else statement here, re-ran tests and it causes all 'inRange' tests to fail except: 'should handle negative numbers as well'
lodash_exercise/lodash.js
Outdated
|
|
||
| if(typeof a[key] === 'object') | ||
| helper(a[key]); | ||
| 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.
we shouldn't need an else return here since our function will end after these lines
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.
have taken out else statement here. will be checked in the next checkin.
| var eKey; | ||
| switch(shape) { | ||
| case "redT": | ||
| clear(ctx, (canvas.width), (canvas.height)); |
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.
If you are doing the clear in all cases, you don't need to put that code in the switch. Just do it before the switch statement. Also, you don't need the ( ) around cavas.width and canvas.height.
|
|
||
| function restartGame(ctx, width, height) { | ||
| function drawSquare(ctx, startx, starty, width, height, color) { | ||
| var square = { |
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 don't think you need to make the whole object because you don't really save it and use it later.
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.
the only code you really need here is:
ctx.fillStyle = color;
ctx.fillRect(startx, starty, width, height);
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.
I didn't see your hacker snooze code. Did you have a chance to submit it?
| person: { | ||
| sayHi: function(){ | ||
| return "This person's name is " + this.fullName | ||
| }.bind(obj) |
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 doesn't work because obj won't be defined yet. Here is a work around:
var obj = {
fullName: "Harry Potter",
person: {
sayHi() {
return `This person's name is ${this.fullName}`;
}
}
}
obj.person.sayHi = obj.person.sayHi.bind(obj);
| } | ||
|
|
||
| function invokeMax(fn, num) { | ||
| var count = num; |
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.
If you count down, you don't even need to copy the variable. Numbers are not reference types.
|
|
||
| Function.prototype.bind = function(thisArg, ...oArgs) { | ||
| var _this = this; | ||
| return function(thisArg) { |
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.
Putting thisArg here would break your bind. Because the local thisArg would override the thisArg from the outer fucntion. And the thisArg from the outer function is the one you want to use to bind.
|
Adding partial hack-snooze working code with login functionality working. Need to refactor code to use variable caching and implement other features. |
No description provided.