Skip to content

London | May 2025 | Houssam Lahlah | Acoursework/sprint 2 #672

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.

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

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

HoussamLh
Copy link

@HoussamLh HoussamLh commented Jul 20, 2025

Check List

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist :

This PR contains solutions for the Sprint 2 coursework exercises:

key-errors:
Implemented capitalise, convertToPercentage, and fixed square function errors.

mandatory-debug:
Debugged and fixed multiply, sum, and getLastDigit functions.

mandatory-implement:
Implemented calculateBMI, converted strings to upper case, and created reusable toPounds function.

mandatory-interpret:
Added formatTimeDisplay function to convert seconds into HH:MM:SS format.

stretch-extend:
Extended formatAs12HourClock function with edge case handling and added tests.

All functions have been tested with relevant inputs to ensure correctness.

Questions

Is my use of descriptive commit messages clear and helpful for reviewers?

Are there any improvements I could make in code style or function structure based on the style guide?

HoussamLh added 20 commits June 17, 2025 23:30
@HoussamLh HoussamLh added the Needs Review Participant to add when requesting review label Jul 20, 2025
@cjyuan
Copy link
Contributor

cjyuan commented Jul 20, 2025

You made some changes to the "Sprint-1" folder in your main branch. As a result, when you created the acoursework/sprint-2 branch from main, it inherited those changes. However, the updates in the "Sprint-1" folder are unrelated to the Sprint-2 exercise.

Can you revert the changes made in the "Sprint-1" folder?
One way to "revert the changes" in the "Sprint-1" folder is to replace the "Spint-1" folder in this branch by the "Sprint-1" folder from CYF's main branch: https://github.com/CodeYourFuture/Module-Structuring-and-Testing-Data/tree/main

You can download CYF's main branch as a ZIP file and then copy/paste the "Sprint-1" folder from the ZIP file to your branch.

Note: Your acoursework/sprint-3 branch is also affected in the similar way.

@cjyuan cjyuan added the 👀 Review Git Changes requested to do with Git label Jul 20, 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.

Code is good and explanation is clear.

Some of the code could use some improvement.

Comment on lines 3 to 4
const pounds = kg * 2.20462;
return pounds.toFixed(2); // rounds to 2 decimal places
Copy link
Contributor

Choose a reason for hiding this comment

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

The pounds refer to the British currency.
This exercise the objective is to rewrite the code in Sprint-1/3-mandatory-interpret/3-to-pounds.js as a function.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the feedback. I understand now that the exercise is about converting pence to British pounds, not kilograms to pounds.

I rewrite the code in Sprint-1/3-mandatory-interpret/3-to-pounds.js as a function that takes a pence string (like "399p") and returns the formatted pounds (like "£3.99"), then test it with different inputs.

Let me know if I’ve understood it correctly.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review labels Jul 20, 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.

Changes look good and good job in fixing the branch.

  • Your forgot to check these checkboxes
image
  • You haven't yet changed the label to "Needs review" but I assumed you have finished changing your code.

Comment on lines +15 to +22
formattedHour = String(hours - 12).padStart(2, "0");
period = "pm";
} else {
formattedHour = String(hours).padStart(2, "0");
period = "am";
}
return `${time} am`;

return `${formattedHour}:${minutes} ${period}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could also consider format the hours with padStart(2, "0") at line 22 (at one location); less code and make code maintenance easier.

@cjyuan cjyuan added Complete Volunteer to add when work is complete and review comments have been addressed and removed 👀 Review Git Changes requested to do with Git Reviewed Volunteer to add when completing a review labels Jul 21, 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 review comments have been addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants