Skip to content
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.

Sign up for GitHub

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

Jump to bottom

PR-1 calculate average method #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

PR-1 calculate average method #1

wants to merge 1 commit into from

Conversation

CheezItMan
Copy link

No description provided.

cescarez
cescarez reviewed Mar 11, 2021
View reviewed changes
calculate_average.js
const calculateAverage = function calculateAverage(grades) {

let total = 0;
for (let i = 0; i < grades.length; i++) {
Copy link

Choose a reason for hiding this comment

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

Can we use a .forEach here instead?

Copy link

Choose a reason for hiding this comment

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

Or a .reduce for the whole code block?

cescarez
cescarez suggested changes Mar 12, 2021
View reviewed changes
Copy link

@cescarez cescarez left a comment

Choose a reason for hiding this comment

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

Looks great! 😄 Threw in some style comments and a question

calculate_average.js
const calculateAverage = function calculateAverage(grades) {

let total = 0;
for (let i = 0; i < grades.length; i++) {
Copy link

Choose a reason for hiding this comment

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

Or a .reduce for the whole code block?

calculate_average.js

let avg = total / grades.length;

return avg;
Copy link

Choose a reason for hiding this comment

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

Can we combine lines 11 and line 9, ie return total/grades.length;?

calculate_average.js
@@ -0,0 +1,14 @@

const calculateAverage = function calculateAverage(grades) {
Copy link

Choose a reason for hiding this comment

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

Can you tell me more why this function is assigned to a constant of the same name?

codeandmorecode
codeandmorecode reviewed Mar 12, 2021
View reviewed changes
Copy link

@codeandmorecode codeandmorecode left a comment

Choose a reason for hiding this comment

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

Lines 5-7 to be revised.
Thanks

calculate_average.js
Comment on lines +5 to +7
for (let i = 0; i < grades.length; i++) {
total += grades[i];
}
Copy link

Choose a reason for hiding this comment

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

Consider changing this loop to a forEach loop for better readibility.

kimvitug05
kimvitug05 reviewed Mar 12, 2021
View reviewed changes
calculate_average.js
@@ -0,0 +1,14 @@

const calculateAverage = function calculateAverage(grades) {
Copy link

Choose a reason for hiding this comment

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

Consider: arrow function syntax ➡️

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

@codeandmorecode codeandmorecode codeandmorecode left review comments

@kimvitug05 kimvitug05 kimvitug05 left review comments

@cescarez cescarez cescarez requested changes

@kaidamasaki kaidamasaki Awaiting requested review from kaidamasaki

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@CheezItMan @cescarez @kimvitug05 @codeandmorecode