Skip to content
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

NW6 | Pedro Ricciardi | JS1 Module | [TECH ED] Complete week 3 exercises | Week 3 Backlog #156

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

Conversation

PERicci
Copy link

@PERicci PERicci commented Nov 21, 2023

Self checklist

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

Changelist

All tasks done. Implemented assertions, improved code using a function to create an assertion pattern, used methods to manipulate strings and numbers, used 'switch' instead of 'if' for cleaner code, used methods to check data type, and worked with Unicode characters.

Copy link

@Dominic-Taylor-Dev Dominic-Taylor-Dev left a comment

Choose a reason for hiding this comment

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

I really like what you've done. A comment I started making and then realised I'd have to repeat it a lot of times, so stopped, is that a lot of the time you are using else when you don't really need to. This is a style point rather than a functional change, but you will probably find your code more readable in the long term if you treat else with suspicion and ask yourself if you really need it.

week-3/debug/format-as-12-hours.js Show resolved Hide resolved
week-3/debug/format-as-12-hours.js Show resolved Hide resolved
week-3/implement/get-angle-type.js Show resolved Hide resolved
week-3/implement/get-card-value.js Outdated Show resolved Hide resolved
week-3/implement/get-card-value.js Outdated Show resolved Hide resolved
week-3/refactor/is-vowel.js Outdated Show resolved Hide resolved
week-3/refactor/is-vowel.js Outdated Show resolved Hide resolved
week-3/stretch/rotate-char.js Outdated Show resolved Hide resolved
week-3/stretch/rotate-char.js Outdated Show resolved Hide resolved
week-3/stretch/rotate-char.js Show resolved Hide resolved
Copy link

@bunday bunday left a comment

Choose a reason for hiding this comment

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

Great job!

@@ -21,3 +21,37 @@
// Identify Reflex Angles:
// When the angle is greater than 180 degrees and less than 360 degrees,
// Then the function should return "Reflex angle"

function getAngleType(angle) {
if (angle >= 360 || angle <= 0) {
Copy link

Choose a reason for hiding this comment

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

360 itself is not an invalid angle

@@ -29,3 +29,46 @@
// Given a card with an invalid rank (neither a number nor a recognized face card),
// When the function is called with such a card,
// Then it should throw an error indicating "Invalid card rank."

function getCardValue(card) {
Copy link

Choose a reason for hiding this comment

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

clean solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants