Skip to content

London|May_2025|Fatma_Degirmenci|Structuring_and_Testing_Data|Sprint1#508

Closed
fatmaevin wants to merge 7 commits intoCodeYourFuture:mainfrom
fatmaevin:coursework/sprint-1
Closed

London|May_2025|Fatma_Degirmenci|Structuring_and_Testing_Data|Sprint1#508
fatmaevin wants to merge 7 commits intoCodeYourFuture:mainfrom
fatmaevin:coursework/sprint-1

Conversation

@fatmaevin
Copy link

@fatmaevin fatmaevin commented Jun 10, 2025

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 read the explanations in Sprint 1 and did the necessary research to learn about JavaScript operators, variable declarations, and various methods like floor and slice. Then I practiced by making some changes in the code.

…ds like slice, random, and floor. Made updates to the code accordingly for the assignment.
@fatmaevin fatmaevin added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jun 10, 2025
@fatmaevin fatmaevin closed this Jun 14, 2025
@fatmaevin fatmaevin reopened this Jun 15, 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"?
    In VSCode, you can select which file to stage, and commit only the staged file.
    See: https://www.youtube.com/watch?v=z5jZ9lrSpqk&t=705 (At around 12:50 minute marker, the video shows how to stage a single file).

  2. When describing code, we should avoid simply translating it literally because:

  • Other developers can already read the code and see what it does.
  • A literal description adds no extra insight to help others understand the purpose, logic, or context of the code.

Here is an example of a literal translation of a piece of code:

   // if num mod 2 is 0
   if (num % 2 == 0)
     ...

A better description would be "if num is even".

Describing code can be quite challenging at first (even in your native language). I suggest using ChatGPT to explore alternative ways to describe the code. Along the way, you might also pick up some useful programming terminology.

paddedPenceNumberString.length - 2
); - In this line, the substring method is used to extract only the first character from the previously created paddedPenceNumberString variable,
which has a length of 3. This extracted part is assigned to pounds.
5- And finally, here we use the substring method to take the last two characters, and then use the padEnd method to add zeros at the end
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?

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jun 22, 2025
@fatmaevin fatmaevin added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 7, 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.

Leaving responses directly in the comment threads makes tracking the discussion easier.

image

Here is a simplified version of best practices ChatGPT suggested for responding to inline comments in a pull request:

  • ✅ Reply to every comment – Let the reviewer know you saw it.
  • ✏️ Make the change if needed – Fix the code if the comment points out a real issue.
  • 🤔 Explain if you don't agree – If you think the code is fine, politely explain why.
  • ✅ Mark as resolved when done – Only mark comments resolved after you fix or respond.
  • 💬 Keep replies short and polite – Be respectful and to the point.
  • ⏱️ Respond soon – Don’t wait too long to reply.
  • 🧪 Test your changes – Make sure your fixes actually work.
  • 📍 Reply directly under the comment – This keeps the conversation easy to follow.

You can try the approach in future PRs.

Comment on lines 41 to 43
5- And finally, here we use the substring method to take the last two characters, and then use the padEnd method to add zeros at the end if
the length is less than 2.padEnd(2, "0") ensures the pence part always has two digits.Without this, if the pence is a single digit (e.g., "5"),
it could be displayed incorrectly (e.g., "5" instead of "50").
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried executing the script without padEnd(2, "0") using different values of penceString to validate your expectation?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I just tested the script with various penceString values without the .padEnd(2, "0") part.
It works correctly in all cases because .padStart(3, "0") already adds leading zeros when needed, ensuring the pence part always has two digits—I realized this just now.
So, .padEnd(2, "0") is actually unnecessary.
I will update the comment to reflect this and remove the misleading explanation.

@cjyuan cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 7, 2025
@fatmaevin fatmaevin added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 7, 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. Reviewed Volunteer to add when completing a review with trainee action still to take. labels Jul 7, 2025
@cjyuan
Copy link
Contributor

cjyuan commented Jul 7, 2025

Great job!

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