Coursework/ Module-STructure-and-Testing / sprint 1 #293
Conversation
Christelle-b
added
the
Needs Review
Christelle-b
changed the title
cjyuan
added
the
Review in progress
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Good job! You seem to understand the code well.
Some descriptions/explanations could use some polishing.
| // Here, Math.rondom creates a random number between 0 to 1 and Math.floor rounds donw to the nearest integer. | ||
| // When creating a random number between 0 and 1, we will multiply it by the (100 - 1 + 1), then round it down to the nearest integer | ||
| // Then add all of that to 1. |
There was a problem hiding this comment.
The phrase "between 0 and 1" alone is not precise enough in program specification because
it does not state clearly whether 0 and 1 are included in the range. Can Math.random() return 0 or 1?
Can you explain the purpose of multiplying the value of Math.random() by (maximum - minimum + 1), and the purpose of ... + minimum in the expression?
What is the range of values that could be assigned to num?
| const HourClockTime = "20:53"; | ||
| const hourClockTime = "08:53"; No newline at end of file |
There was a problem hiding this comment.
Having two variable names different only by one character can be confusing. Also, it is a common practice to start a variable name by a lowercase letter.
Can you think of some better names for them?
| //There are two function calls here. replaceAll and console.log | ||
|
|
There was a problem hiding this comment.
There are more than two function calls. :)
Note: foo(bar()) would be considered as two function calls in which bar() is called first and its return value is passed as a parameter to foo().
| // Line 1, line 2, line 7 and line 8 have variable declarations | ||
|
|
||
| // e) Describe what the expression Number(carPrice.replaceAll(",","")) is doing - what is the purpose of this expression? | ||
| //This expression removes the comma in the price from 10,000 to 10000 |
There was a problem hiding this comment.
If carPrice is "10,000", then carPrice.replaceAll(",","") returns "10000". What does Number(...) do?
| // % is a remainder operator which will do movieLength / 60 and we will consider the remainder of this operation | ||
|
|
||
| // d) Interpret line 4, what does the expression assigned to totalMinutes mean? | ||
| // In line 4 we are declaring the totalMinutes. we have the remainingSeconds which is 24. Then we will do ((8784 - 24) /60) = 146 |
There was a problem hiding this comment.
So what does the value 146 represents?
| // In line 4 we are declaring the totalMinutes. we have the remainingSeconds which is 24. Then we will do ((8784 - 24) /60) = 146 | ||
|
|
||
| // e) What do you think the variable result represents? Can you think of a better name for this variable? | ||
| // The variable result shows the total remaining duration of a movie in terms of hours, minutes and seconds |
There was a problem hiding this comment.
I think you missed the second question at line 27: Can you think of a better name for this variable?
| Answer the following questions: | ||
|
|
||
| What does `console` store? | ||
| Answer: Console stores a lot of objects like assert, clear, error, info, warn, log, debug and more. |
There was a problem hiding this comment.
Note: Those objects happened to be "Function objects" or functions. A function is a type of object in JavaScript.
cjyuan
added
Reviewed
3 participants
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.