Skip to content

Sheffield | May-2025 | Declan Williams | Sprint-3 #623

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 40 commits into
base: main
Choose a base branch
from
Open

Sheffield | May-2025 | Declan Williams | Sprint-3 #623

wants to merge 40 commits into from
+251 −18

Conversation

CatchingKiyoko
Copy link

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

  1. Implemented changes and fixes to files.
  2. added comments detailing ,explaining code and answered questions with comments.
  3. refactored some lines to make neater and easier to read.
  4. added tests with jest.

Briefly explain your PR.

Questions

Ask any questions you have for your reviewer.

@CatchingKiyoko
Implement acute angle identification in getAngleType function and add…
be25f2c 
… corresponding test case
@CatchingKiyoko
Fix syntax error in acute angle test case
4fa86db
@CatchingKiyoko
Implement angle type identification for obtuse, straight, and reflex …
d317d4f 
…angles in getAngleType function
@CatchingKiyoko
Add tests for identifying obtuse angles in getAngleType function
dea34f8
@CatchingKiyoko
Add test cases for identifying reflex angles in getAngleType function
952fd14
@CatchingKiyoko
Implement acute angle identification in getAngleType function
8cf3f58
@CatchingKiyoko
Implement obtuse angle identification in getAngleType function and ad…
6da0615 
…d corresponding test case
@CatchingKiyoko
Implement straight angle identification in getAngleType function and …
45209eb 
…add corresponding test case
@CatchingKiyoko
Implement reflex angle identification in getAngleType function and ad…
37f345d 
…d corresponding test case
@CatchingKiyoko
Implement card value retrieval for face cards and number cards in get…
ed4eaf3 
…CardValue function with comments explaining function and updated tests
@CatchingKiyoko
Implement numerical value retrieval for cards 2-9 in getCardValue fun…
a0d4fcf 
…ction and add corresponding test cases
@CatchingKiyoko
Enhance getCardValue function comments and add error handling for inv…
d32c392 
…alid card ranks
@CatchingKiyoko
Fix typo in error message assertion for invalid card rank in getCardV…
e7f3d74 
…alue function
@CatchingKiyoko
Implement card value retrieval for all ranks in getCardValue function
2e2d63c
@CatchingKiyoko
Add tests for number card values 2-10 in getCardValue function
900385e
@CatchingKiyoko
Add tests for face cards (J, Q, K) in getCardValue function
67c92c2
@CatchingKiyoko
add tests for handling Aces in getCardValue function
60ed74f
@CatchingKiyoko
fix: correct expect statement for Ace value in getCardValue tests
bcdba1c
@CatchingKiyoko
add test for handling invalid card ranks in getCardValue function
a16c211
@CatchingKiyoko
refactor: implement character counting logic in countChar function
aced4d1
@CatchingKiyoko
add test for handling no occurrences of a character in countChar func…
0a913b0 
…tion
@CatchingKiyoko
refactor: implement getOrdinalNumber function to return correct ordin…
de7632b 
…al suffixes
@CatchingKiyoko
reimplmented module.exports for jest tests
aa7b3ee
@CatchingKiyoko
fix: correct ordinal suffix casing in getOrdinalNumber function
d2a8646
@CatchingKiyoko
refactor: remove console.log statements from getOrdinalNumber function
9798ed9
@CatchingKiyoko
test: add test cases for ordinal numbers 2 and 3 in getOrdinalNumber …
63a66e2 
…function
@CatchingKiyoko
test: enhance test cases for ordinal numbers 1, 2, and 3 in getOrdina…
c89c3b8 
…lNumber function
@CatchingKiyoko
fix: correct conditional check for special cases in getOrdinalNumber …
2b44bf7 
…function
@CatchingKiyoko
test: update test descriptions for ordinal numbers and implemented a …
df5287d 
…test for special ordinal numbers
@CatchingKiyoko
implemented repeat function with error handling and edge cases with …
1f02ed6 
…comments detailing.
@CatchingKiyoko CatchingKiyoko linked an issue Jun 28, 2025 that may be closed by this pull request
02 Complete Sprint 3 coursework CatchingKiyoko/My-Coursework-Planner#21
Open
@CatchingKiyoko CatchingKiyoko added 📅 Sprint 3 Assigned during Sprint 3 of this module Needs Review Participant to add when requesting review labels Jun 28, 2025
cjyuan
cjyuan reviewed Jul 3, 2025
View reviewed changes
Sprint-3/2-mandatory-rewrite/1-get-angle-type.js
Comment on lines +4 to 12
if (angle < 90)
return "Acute angle";
if ((angle > 90) && (angle < 180))
return "Obtuse angle";
if (angle === 180)
return "Straight angle";
if ((angle > 180) && (angle < 360))
return "Reflex angle";
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Your function can return undefined for some values of angle. To ensure reliability, please update the function so it always returns a defined value (which can be "Invalid angle") or throws an error. Functions that are expected to return a result should never return undefined.

Note: the spec fails to mention angles <= 0 are not "Acute angle".

Sprint-3/2-mandatory-rewrite/2-is-proper-fraction.js
Copy link
Contributor

Choose a reason for hiding this comment

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

In mathematics, -4/7 == 4/-7, and -4/-7 == 4/7.
So, ideally isProperFraction() should recognise all of them as proper fractions.

Similarly, -5/2 == 5/-2, and -5/-2 == 5/2.
So isProperFraction(-5, 2) should also return false because -5/2 is not a proper fraction. (Currently your function return true)

Hint: we can compare the absolute value of both parameters instead.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review labels Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjyuan cjyuan cjyuan left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Reviewed Volunteer to add when completing a review 📅 Sprint 3 Assigned during Sprint 3 of this module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

02 Complete Sprint 3 coursework
2 participants
@CatchingKiyoko @cjyuan