Skip to content

London | May 2025 | Houssam Lahlah | Acoursework/sprint 3 #678

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

Self checklist

  • 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: https://curriculum.codeyourfuture.io/guides/reviewing/style-guide/
  • My changes meet the requirements of this task

Changelist

  • Implemented getOrdinalNumber, repeat, countChar functions with tests

  • Completed credit card validator with all validation rules

  • Completed password validator basic structure (to be improved for full requirements)

  • Added comments to explain logic and decisions in each file

Questions

For the password validator, should we include all rules in one function or break them into smaller helper functions for readability?

Can you confirm if my approach for repeat() with error handling on negative count is acceptable?

HoussamLh and others added 25 commits June 17, 2025 23:30
Added comments to exclude instructional lines from execution.
Updated age variable to let to allow incrementing its value
Declare variable before using in template string.
Add comments explaining slice error and fix by converting number to string
…umerator cases

and add stretch scenarios for isProperFraction edge case testing
- Return proper suffixes for 1st, 2nd, 3rd, and others (e.g., 4th, 11th, 23rd)
- Added Jest tests for various edge cases including teens (11th-13th)
- Improved function to handle general inputs dynamically
- Added str and count parameters to repeat function
- Used String.repeat to repeat str count times
- Throws error if count is negative
- Added tests for count = 1, 0, and negative values
@HoussamLh HoussamLh added the Needs Review Participant to add when requesting review label Jul 21, 2025
@kfklein15 kfklein15 removed the Needs Review Participant to add when requesting review label Jul 22, 2025
@HoussamLh HoussamLh added the Needs Review Participant to add when requesting review label Jul 22, 2025
Copy link

@LonMcGregor LonMcGregor left a comment

Choose a reason for hiding this comment

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

Good work on this sprint. To answer your questions: In a larger function you would want to split the code up to make it easier to read and more maintainable, but I think the file in this sprint is small enough that it doesn't matter so much. Your repeat function is fine. You can close this PR now.

@@ -8,7 +8,9 @@
// write one test at a time, and make it pass, build your solution up methodically

function isProperFraction(numerator, denominator) {
// Use absolute value of numerator to handle negatives
if (numerator < denominator) return true;

Choose a reason for hiding this comment

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

I can see from your later implementation in section 2 you got this right. If you made changes, remember to copy them over

@LonMcGregor LonMcGregor removed the Needs Review Participant to add when requesting review label Jul 23, 2025
@LonMcGregor LonMcGregor added the Complete Volunteer to add when work is complete and review comments have been addressed label Jul 23, 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.

3 participants