Manchester | 26-ITP-JAN | Abdu Mussa | Sprint 1 | structuring and Testing Data #1058
Manchester | 26-ITP-JAN | Abdu Mussa | Sprint 1 | structuring and Testing Data #1058Abduhasen wants to merge 8 commits intoCodeYourFuture:mainfrom
Conversation
…aracter of each string
Abduhasen
added
📅 Sprint 1
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Have you installed prettier VSCode extension and enable formatting on save/paste on VSCode as recommended in
https://github.com/CodeYourFuture/Module-Structuring-and-Testing-Data/blob/main/readme.md ?
| //This expression calculates the total full minutes in the movie by removing leftover seconds and converting seconds to minutes. | ||
|
|
||
| // e) What do you think the variable result represents? Can you think of a better name for this variable? | ||
| //result represent the total movie length using hours,minutes and seconds,the better name can be totalMovieLength. |
There was a problem hiding this comment.
The name totalMovieLength does not quite indicate the value the variable stores is a formatted string in the form "2:12:02".
Can you suggest a more descriptive name?
There was a problem hiding this comment.
thank you for your feedback i changed it to totalMovieDuration.
There was a problem hiding this comment.
totalMovieDuration still does not quite describe the value that is a string formatted in form "02:12:02".
Numbers such as 2.2 (hours) and 132.03 (minutes) are also valid total movie duration.
One more try?
There was a problem hiding this comment.
how about movieRunTime this one can describe it more.
There was a problem hiding this comment.
Not much difference from movieDuration. You could try asking an AI tool for suggestion.
| /* 5. const pence = paddedPenceNumberString | ||
| .substring(paddedPenceNumberString.length - 2) | ||
| .padEnd(2, "0"); : let's break it down this code parts it have two steps first one is .substring(paddedPenceNumberString.length - 2) | ||
| Takes the last 2 characters of the string and the second is .padEnd(2, "0") Ensures the result | ||
| is at least length 2 if the length less than 2 it will add 0 at the end of the string. */ |
There was a problem hiding this comment.
Optional challenge:
Could we expect this program to work as intended for any valid penceString if we deleted .padEnd(2, "0") from the code?
In other words, do we really need .padEnd(2, "0") in this script?
There was a problem hiding this comment.
thank you for the feedback, we don't need the .padEnd(2, "0") in this script because the program will work for any valid penceString even after removing the padEnd.
There was a problem hiding this comment.
Did you figure out why .padEnd(2, "0") is optional in this script? It is more important to understand why.
There was a problem hiding this comment.
yes i understood the concept before i didn't realise it ( which is even if we didn't put the pading there will be a value of 2 character and we don't need to have extra script. )
i also want to thank you because i learned more by the feedback i got from you.
There was a problem hiding this comment.
I am glad you understand the concept.
Can you explain why .padEnd(2, "0") is optional in this script?
cjyuan
added
Reviewed
Abduhasen
added
the
Needs Review
|
Changes to the code look good. Can you explain why
This description (In the PR description) is too general. Can you give a more specific description of what you have changed? |
cjyuan
removed
the
Needs Review
Abduhasen
added
the
Needs Review
|
Changes to the code are good. Would be better if you can also address the comments in the two unresolved conversations. I will mark this PR as "Complete" first. |
cjyuan
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
this PR is for the Structuring and Testing Data.
-working on errors identifying them and fixing the errors with explanation
-brief explanation of codes and interpret
Questions
N/A