Skip to content

Eddie bickham - #1

Open
Eddeh1827 wants to merge 2 commits into
masterfrom
eddie-bickham
Open

Eddie bickham#1
Eddeh1827 wants to merge 2 commits into
masterfrom
eddie-bickham

Conversation

@Eddeh1827

Copy link
Copy Markdown
Owner

No description provided.

@Eddeh1827
Eddeh1827 requested a review from clem9281 May 8, 2019 10:23

@clem9281 clem9281 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3:star:
Awesome work, fantastic stretches. Great job with the counter factory. I have a couple small suggestions, but overall it's great work!

Comment thread assignments/callbacks.js
// getLength passes the length of the array into the callback.
return cb(arr.length);
}
getLength(items, console.log);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
getLength(items, console.log);
getLength(items, (item) => console.log(item));

You have supply a callback here, follow this trend down the page

Comment thread assignments/closure.js

/* STRETCH PROBLEMS, Do not attempt until you have completed all previous tasks for today's project files */
function sayWorld() {
console.log(greet, "World!");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
console.log(greet, "World!");
return `${greet}, World!`

Nice closure, but I recommend doing this instead here

Comment thread assignments/closure.js
return sayWorld;
}
let hi = sayHello();
hi();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
hi();
console.log(hi());

And now you can see this working a little better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants