-
Notifications
You must be signed in to change notification settings - Fork 0
Second challenge #2
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
25e0b22 to
3c1e335
Compare
3c1e335 to
ef740e4
Compare
5cb1605 to
9ab399e
Compare
oscarpolanco
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!!
Some comments that we can discuss
script.js
Outdated
| let mikeTeamScores = [116,94,123]; | ||
| let johnsAvrg = calcAverage(johnTeamScores); | ||
| let mikesAvrg = calcAverage(mikeTeamScores); | ||
| let hasHigherScore = johnsAvrg > mikesAvrg? "John's team has a higher average of " + johnsAvrg : "Mike's team has a higher average of " + mikesAvrg; |
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.
Missing spaces 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.
I think we can update the conditions, but first I would like to read if you have another approach to change these conditions.
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 other option i can think of is to save the possible results in variables so the code is easier to read.
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.
Can we store the winner on some kind of structure like an object so we don't need to repeat the string so many times?
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.
Changed it, is it fine like this?
script.js
Outdated
| let johnsAvrg = calcAverage(johnTeamScores); | ||
| let mikesAvrg = calcAverage(mikeTeamScores); | ||
| let hasHigherScore = johnsAvrg > mikesAvrg? "John's team has a higher average of " + johnsAvrg : "Mike's team has a higher average of " + mikesAvrg; | ||
| let avrg = johnsAvrg == mikesAvrg? "John's and Mike's teams have the same average score" : hasHigherScore; |
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.
Missing space here
script.js
Outdated
| let maryAvrg = calcAverage(maryTeamScores); | ||
| let winner = "The three teams are tied"; | ||
|
|
||
| if (maryAvrg > johnsAvrg && maryAvrg> mikesAvrg) { |
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.
Missing space here
script.js
Outdated
| scores.forEach(score => { | ||
| sum += score; | ||
| }); | ||
| return sum/scores.length; |
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.
Missing space here
script.js
Outdated
| @@ -0,0 +1,35 @@ | |||
| let johnTeamScores = [89,20,103]; | |||
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 you should re-check this definition.
Can you give me some inside about let, const and var?
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 main difference between var and let is that let is block-scoped instead of function-scoped, which is considered a better coding practice. Let also doesn't allow you to access a variable before its declared.
On the other hand, const is similar to let, the difference being that you can't assign a new value to the variable.
I realized I should have declared the scores as const since I don't need to change these values. Fixed the code.
script.js
Outdated
| @@ -0,0 +1,35 @@ | |||
| let johnTeamScores = [89,20,103]; | |||
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.
Missing spaces here
script.js
Outdated
| @@ -0,0 +1,35 @@ | |||
| let johnTeamScores = [89,20,103]; | |||
| let mikeTeamScores = [116,94,123]; | |||
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.
Missing spaces here
script.js
Outdated
|
|
||
| //Extra | ||
|
|
||
| let maryTeamScores = [97,134,105]; |
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.
Missing spaces here
86428b0 to
81dd312
Compare
33dedfe to
d6134d7
Compare
script.js
Outdated
| higherAverage.highestAvrg = ''; | ||
| } | ||
|
|
||
| const avrg = higherAverage.winner + higherAverage.highestAvrg + higherAverage.score; |
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.
On Es6 were introduce the template literals. We can use it here instead of storing the complete string value; just need to put it twice in case there was a tie.
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.
Like this?
|
|
||
| higherAverage.setWinner(); | ||
|
|
||
| function calcAverage(scores) { |
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.
c08f800 to
1603edf
Compare
1603edf to
f3be13a
Compare
Calculates which team has the highest average score and prints the result in the console.
The extra part of this challenge consisted on adding a third team and determining which of the 3 has the highest average score.
Verification steps
Download both files(index.html and script.js) into a folder and then open the index.html file on a browser.
Inspect the website and open the console to see the result obtained.