Skip to content

Conversation

@Zolton
Copy link
Owner

@Zolton Zolton commented May 7, 2019

Hard stuff. Could have easily spent an entire day on callbacks alone, or array methods alone.

@Zolton Zolton requested a review from OmarSalah95 May 7, 2019 23:09
Copy link
Collaborator

@OmarSalah95 OmarSalah95 left a comment

Choose a reason for hiding this comment

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

Good overall work using the logic and applying the callbacks and so on, I have noticed your naming convention has gotten abit crazy, with functions call haha, and parameters called lol.

You need to keep your naming conventions consistent so that when other devs reading your code can make sense of it.

Don't forget when it comes to parameters. you can use the same name in multiple functions. So if you have a filter, map, and reduce in 3 separate spots, and they all iterate through the same runners array, just have all of the parameters (aka place holders) be runner, there is no need to invent new non descriptive placeholders


let fullName2 = []
runners.forEach(runner => fullName2.push(`${runner.first_name} ${runner.last_name}`))
console.log(fullName2);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great job using Template string literals here to render the name information from the runner object. This could also be done using string concatenation as well.

runners.forEach(addName)

function addName(word){
fullName4.push(`${word.first_name} ${word.last_name}`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice abstracting the callback into its own function rather than typing it in the forEach callback.

// for (let i = 0; i < runners.length; i++) {
// fullName.push(runners[i].first_name + " " + runners[i].last_name)
// }
// console.log(fullName)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like seeing that you got both syntax's here.


function allCapMode (words) {
return words.first_name.toUpperCase()
// allCaps.push(`${wubba.first_name}`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Glad to see it looks like you have a fairly solid grasp on callbacks now!



let largeShirts = runners.filter(shirts).map(names)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice use of Method chaining here, using dual abstracted callback functions stored in the global scope as their arguments!

function names(xxx) {
return xxx.first_name + " - " + xxx.shirt_size
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Naming convention! what is the xxx supposed to be a place holder for? is it not each item in the array we are mapping through?

Because of method chaining we. in this case, are iterating through the freshly filtered array of runners with a shirt size of large.

So this xxx parameter is a place holder for a runner object who already meets the conditions of being a large shirt size wearer


function sums(x, y) {
return x + y.donation
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

In reducers, you are better off sticking to keeping the actual callback inside of your reduce and in 1 unit.


function tally (jolly) {
return jolly.donation
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Naming convention.

function dollars (cash) {
cashflow.push(`To: ${cash.email} <br><br> Hello, Mr or Ms ${cash.first_name} ${cash.last_name}, we have an exciting offer for you today`)
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

the breaks are HTML tags not JS .

function haha(lol) {
if (lol.donation > 150)
return lol.first_name + lol.last_name + lol.email + lol.donation
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is happening here with these names?!?!

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.

3 participants