Skip to content

Conversation

@avatarit
Copy link

@avatarit avatarit commented Jun 4, 2025

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with REGION | COHORT_NAME | FIRST_NAME LAST_NAME | PROJ_NAME
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

I have solved all issues in sprint 1

@avatarit avatarit added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jun 4, 2025
@avatarit avatarit requested a review from shieldo June 4, 2025 22:48
@cjyuan cjyuan added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jun 16, 2025
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

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

  1. Why not practice "committing files one by one, on purpose, and for a reason"?
  1. Explanations are clearly written. I only have few suggestions and a question.

Comment on lines +8 to +10
// Line 3 is updating the value of the count variable by adding 1 to its current value. The = operator is used for assignment, meaning it takes the value on the right (count + 1) and assigns it to the variable on the left (count).
// The expression count + 1 calculates the new value, and then the = operator assigns this new value back to the count variable.
// The count variable now holds the value 1, which is the result of the initial value (0) plus 1.
Copy link
Contributor

Choose a reason for hiding this comment

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

Operation like count = count + 1 is very common in programming, and there is a programming term describing such operation.

Can you find out what one-word programming term describes the operation on line 3?

Copy link
Author

Choose a reason for hiding this comment

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

Yes count++;

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the programing term for such operation?

Comment on lines +12 to +13
// The variable num represents a random integer between the values of minimum (1) and maximum (100), inclusive.
// The expression Math.random() generates a random floating-point number between 0 (inclusive) and 1 (exclusive).
Copy link
Contributor

Choose a reason for hiding this comment

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

You can also use the concise and precise interval notation to describe a range of values.
For example, we can say Math.random() returns a random number in the interval [0, 1).

Comment on lines +25 to +27
// a) There are 2 function calls in this file:
// 1. `carPrice.replaceAll(",", "")` on line 4
// 2. `priceAfterOneYear.replaceAll(",", "")` on line 5
Copy link
Contributor

Choose a reason for hiding this comment

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

There are more than 2 function calls.

Copy link
Author

@avatarit avatarit Jun 18, 2025

Choose a reason for hiding this comment

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

Yes, I have added the 3rd function

Copy link
Contributor

Choose a reason for hiding this comment

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

There are more then 3 function calls.

I could not see any new changes made to the file Sprint-1/3-mandatory-interpret/1-percentage-change.js.

Comment on lines +35 to +36
// .substring(paddedPenceNumberString.length - 2).padEnd(2, "0"):
// extracts the last two characters (the pence part) and pads it with trailing zeros to ensure it has 2 characters, resulting in "99"
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Author

Choose a reason for hiding this comment

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

I have refactored the code for improved readability and remove unnecessary padding logic
const pounds = paddedPenceNumberString.substring(0, paddedPenceNumberString.length - 2);
No need to padEnd here since paddedPenceNumberString always has at least 3 digits

Copy link
Contributor

Choose a reason for hiding this comment

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

Your understanding is correct. Change is not needed. The task only asks you to describe how the code works.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Jun 16, 2025
@avatarit avatarit added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Jun 28, 2025
@cjyuan cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jun 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants