London 11 | Pezhman-Azizi | Structuring-and-testing-data | week3 | sprint 3 #214
Conversation
There was a problem hiding this comment.
I left my comments and suggestions in the code.
To answer your questions:
Q: For rotateCharacter.js, do you think the way I’m handling big negative shifts is solid, or should I try something different?
A: When you test your function on large negative shift values, does your function return the value you expected?
Q: In passwordValidator.js, the repeated password check uses a fixed list right now—would it make more sense to handle this dynamically somehow?
A: This is just a programming exercise. If something is not specified clearly in the spec, you can make appropriate assumption. So whether to update your password array dynamically is up to you.
In practice, web applications don't save user's passwords in the database.
Q: In countChar.js, the function assumes case sensitivity—should I consider adding an option for case-insensitive searches, or does it overcomplicate things?
A: Up to you. :)
| return "invalid angel"; | ||
| } | ||
| } | ||
| console.log(getAngleType(180)); |
There was a problem hiding this comment.
Only one test case?
Without using Jest, you can prepare your test as
console.assert(getAngleType(180) === "Right angle", "Right angle");
so that if the function does not return the expected value, you will see the error message "Right angle".
Can you include more test cases to thoroughly test your function? They can help you detect bugs in your code.
| }else | ||
| return `Invalid card rank.` | ||
| } | ||
| console.log(getCardValue("K♠")); |
There was a problem hiding this comment.
What do you expect from the following function calls?
Does your function returns the value you expected?
getCardValue("0Q♠");
getCardValue("010♠");
getCardValue("02♠");
getCardValue("0x02♠");
getCardValue("2.1♠")
|
|
||
| if(denominator === 0){ | ||
| throw new Error("Denominator cannot be zero"); | ||
| }else if(numerator === denominator){ |
There was a problem hiding this comment.
This test, if (numerator === denominator), is optional. Can you figure out why?
| console.log(isProperFraction(4, 0)); | ||
|
|
||
|
|
||
| console.assert(isProperFraction(2, 3) === true, "2/3 should be a proper fraction"); |
There was a problem hiding this comment.
Could also test negative values in numerator and/or denominator (for cases when |numerator| < |denominator|).
|
|
||
| function isValidTriangle(a, b, c){ | ||
| // Checks if the sides are positive | ||
| if (a <= 0 || b <= 0 || c <= 0){ |
There was a problem hiding this comment.
Is it necessary to include the if-statement at lines 42-44?
Can you find any values for a, b, and c, such that the function will fail after you removed the if-statement at lines 42-44?
If you cannot find such a, b, and c, that means you probably do not need that if-statement.
I will not go into details why in some programming languages (but not JavaScript) we need also to check if a, b, c are positives.
The main point I would like to make is, you should fully understand what you wrote in your code. An interviewer may ask you questions like what I am asking here, and it would reflect poorly on you if you cannot explain your code.
| count += 1; | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
Maybe the spec wasn't clear enough. Usually a function that "count something" would simply return an integer.
I am afraid you may have overdone.
| @@ -0,0 +1,21 @@ | |||
| function getOrdinalNumber(number){ | |||
|
|
|||
| let firstDigit = number%10; | |||
There was a problem hiding this comment.
A better practice is to use const instead of let to declare variables which do not change. Such practice could lead to less error prone code.
|
|
||
| if(number<=1) return false; | ||
|
|
||
| for (let i = 2; i <= number-1; i++) { |
There was a problem hiding this comment.
To check if number has a factor that can fully divide it, you can check only odd numbers, and only odd numbers less than or equal to square root number.
Doing so can possibly improve the performance of the function.
You can try Google to find out "how to determine if a number is prime efficiently in JS".
| let hasNumber = (/\d/.test(password)); | ||
| let hasSpecialCharacter = (/[!#$%.*&]/.test(password)); | ||
| let repeated = passwords.includes(password); | ||
| console.log(repeated); |
| }else if (int === 1 ){ | ||
| return str; | ||
| }else{ | ||
| return str.repeat(int); |
There was a problem hiding this comment.
What are the values of str.repeat(1) and str.repeat(0)?
cjyuan
added
Reviewed
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
This pull request includes all the core functions and tests needed for Sprint-3. I’ve added a bunch of JavaScript implementations (like handling angles, cards, fractions, triangles, etc.) and wrote tests for everything using Jest. Also updated the package.json to set up testing.
Questions
For rotateCharacter.js, do you think the way I’m handling big negative shifts is solid, or should I try something different?
In passwordValidator.js, the repeated password check uses a fixed list right now—would it make more sense to handle this dynamically somehow?
In countChar.js, the function assumes case sensitivity—should I consider adding an option for case-insensitive searches, or does it overcomplicate things?