West Midlands | Gabriel Deng | Module-Structuring-and-Testing-Data | Sprint 3

[gai93003]

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 COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
• I have tested my changes
• My changes follow the style guide
• My changes meet the requirements of this task

Changelist

Briefly explain your PR.

Questions

Ask any questions you have for your reviewer.

[cjyuan]

You have modified files from Sprint-1 and Sprint-2 in your branch. Can you rebase your Sprint-3 branch to main so that the files in Sprint-1 and Sprint-2 remains the same as those in main?

[gai93003]

I have tried rebasing by following a tutorial using the CLI, please let me know if the rebasing have work as intended.

…

On Mon, 2 Dec 2024 at 00:49, CJ Yuan ***@***.***> wrote: You have modified files from Sprint-1 and Sprint-2 in your branch. Can you rebase your Sprint-3 branch to main so that the files in Sprint-1 and Sprint-2 remains the same as those in main? — Reply to this email directly, view it on GitHub <#207 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BC3JQE5IADQAG5YJRENMT2T2DOVAHAVCNFSM6AAAAABSV6FAWCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMJQGM2DOMBWGM> . You are receiving this because you authored the thread.Message ID: <CodeYourFuture/Module-Structuring-and-Testing-Data/pull/207/c2510347063@ github.com>

[cjyuan]

Did you run git push --force origin Your_sprint_branch_name to force your local change onto remote repository after you rebased your sprint 3 branch? I can still see the changed files in sprint 1 and 2 folder in your sprint 3 branch. We can try to resolve this on Slack or over huddle if you can post a question or DM me on Slack.

[gai93003]

Alright, I will DM you on slack.

…

On Tue, 3 Dec 2024 at 00:01, CJ Yuan ***@***.***> wrote: Did you run git push --force origin Your_sprint_branch_name? to force your local change onto remote repository after you rebased your sprint 3 branch? I can still see the changed files in sprint 1 and 2 folder in your sprint 3 branch. We can try to resolve this on Slack or over huddle if you can post a question or DM me on Slack. — Reply to this email directly, view it on GitHub <#207 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BC3JQE3YAQQPMT67KACY6XD2DTYFBAVCNFSM6AAAAABSV6FAWCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMJTGIZDMNZTGQ> . You are receiving this because you authored the thread.Message ID: <CodeYourFuture/Module-Structuring-and-Testing-Data/pull/207/c2513226734@ github.com>

[cjyuan]

I left my comments in the code. If you need assistance on using Jest, please let me know or seek assistance from a volunteers.

[cjyuan]

How value do you think the function should return when an angle < 0 and or > 360?

[cjyuan]

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

If you can calculate the absolute values of a the parameters first, you can possibly simplify your code.

[gai93003]

I have added the absolute method to the function.

[cjyuan]

Is it necessary to include the if-statement at lines 41-43?
Can you find any values for a, b, and c, such that the function will fail after you removed the if-statement at lines 41-43?
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 ensure a, b, c are positives.

The main point I would like to make is, you should fully understand and be able to explain 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.

Also (a + b <= c || a + c <= b || b + c <= a) == !((a + b > c || a + c > b || b + c > a). These two conditions are opposite of each other. If you are interested in learning more about how to find the opposite of a condition, you can check out De Morgan's Law.

[gai93003]

I was trying to fulfill the acceptance criteria
// scenario: invalid triangle
// Check for Valid Input:
// Given the sides a, b, and c,
// When any of the sides are less than or equal to zero,
// Then it should return false because a triangle cannot have zero or negative side lengths.

[cjyuan]

How should you deal with cases where num is a number like 100?
What if num can also be a negative number?

[cjyuan]

I wonder what your function will return when it is called with the following parameters:

console.log(rotateCharacter("x", 3)); 
console.log(rotateCharacter("Y", 1));  

You can try making your function to work on lowercase letters first, and then duplicate your code with tiny modification to apply them to uppercase letters.

Also, try taking advantage of the % operator so that you don't to use any loop in your solution.

[gai93003]

console.log(rotateCharacter("x", 3)); - returns 'A'
console.log(rotateCharacter("Y", 1)); - returns a

I will look into learning more the operator. Thanks for the recommendation.

[cjyuan]

Are we testing prime number ?

[gai93003]

Not really, i failed to change the description on the test.

[cjyuan]

These tests does not check if the function can correctly return false for each specific type of invalid passwords.
For example, the test only check if the function can return false when a password does not contain any uppercase letter, but the test does not check if the function can return false when a password is missing a lowercase.

Can you include more test cases to see if the function can return false for each type of invalid passwords? They could possibly help you detect a bug in your code.

[gai93003]

I missed out the digit checks in the regular expressions.

[cjyuan]

According to the spec, what should happen if count is a negative number?

[cjyuan]

What do you expect from the following function calls?
Does your function return the value you expected?

getCardValue("0Q♠");
getCardValue("010♠");
getCardValue("02♠");
getCardValue("0x02♠");
getCardValue("2.1♠")

[gai93003]

The function will throw an error 'Invalid card rank since' since the above numbers doesn't match the numbers between 2 and 10. in a blackjack.

[cjyuan]

I think you missed this description:

// 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."

[gai93003]

Thanks alot for the review, i will work on all the areas you suggested and i will get back to you. I would appreciate if you could walk me through jest and the use of assertions in node js whenever you have time.

…

On Sat, 7 Dec 2024 at 00:40, CJ Yuan ***@***.***> wrote: ***@***.**** commented on this pull request. I left my comments in the code. If you need assistance on using Jest, please let me know or seek assistance from a volunteers. ------------------------------ In Sprint-3/implement/get-angle-type.js <#207 (comment)> : > + return "Right angle"; + } + else if(angle < 90) { + return "Acute angle"; + } + else if(angle > 90 && angle < 180) { + return "Obtuse angle"; + } + else if(angle === 180) { + return "Straight angle"; + } + else if(angle > 180 && angle < 360) { + return "Reflex angle"; + } +} + How value do you think the function should return when an angle < 0 and or > 360? ------------------------------ In Sprint-3/implement/is-proper-fraction.js <#207 (comment)> : > + return true; + } + else if (numerator > denominator) { + return false; + } + else if (numerator === denominator) { + return false; + } +} + +console.assert(isProperFraction(3, 0) === false, `No target output: Error (Denominator cannot be zero), Explanation: The function should throw an error when the denominator is zero, as it's not a valid fraction.`); + +// console.assert(isProperFraction(-4, 7), `target output: false, Explanation: The fraction is not a proper fraction because the numerator is equal to the denominator. The function should return false.`); + +console.assert(isProperFraction(-4, 7) === true, `target output: false, Explanation: The fraction is a proper fraction because the absolute value of the numerator (4) is less than the denominator (7). The function should return true.`); + In mathematics, -4/7 == 4/-7 and -4/-7 == 4/7. So, ideally isProperFraction() should recognise all of them as proper fractions. If you can calculate the *absolute values* of a the parameters first, you can possibly simplify your code. ------------------------------ In Sprint-3/implement/is-valid-triangle.js <#207 (comment)> : > @@ -33,3 +33,17 @@ // Then it should return true because the input forms a valid triangle. // This specification outlines the behavior of the isValidTriangle function for different input scenarios, ensuring it properly checks for invalid side lengths and whether they form a valid triangle according to the Triangle Inequality Theorem. + +function isValidTriangle(a, b, c) { + if (a + b <= c || a + c <= b || b + c <= a) { + return false; + } + else if (a <= 0 || b <= 0 || c <= 0) { Is it necessary to include the if-statement at lines 41-43? Can you find any values for a, b, and c, such that the function will fail after you removed the if-statement at lines 41-43? 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 ensure a, b, c are positives. The main point I would like to make is, you should fully understand and be able to explain 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. Also (a + b <= c || a + c <= b || b + c <= a) == !((a + b > c || a + c > b || b + c > a). These two conditions are opposite of each other. If you are interested in learning more about how to find the opposite of a condition, you can check out *De Morgan's Law*. ------------------------------ In Sprint-3/implement/rotate-char.js <#207 (comment)> : > @@ -41,3 +41,36 @@ console.log(rotateCharacter("7", 5)); // Output: "7" (unchanged, not a letter) // And the function should return the rotated character as a string (e.g., 'z' rotated by 3 should become 'c', 'Z' rotated by 3 should become 'C'). console.log(rotateCharacter("z", 1)); // Output: "a" (preserves case, but wraps around) console.log(rotateCharacter("Y", 2)); // Output: "A" (preserves case, but wraps around) + +let lowerCaseLetters = ['a','b','c','d','e','f','g','h','i','j','k','l','m','n','o','p','q','r','s','t','u','v','w','x','y','z']; +let upperCaseLetters = ['A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z']; + +function rotateCharacter(char, num) { How should you deal with cases where num is a number like 100? What if num can also be a negative number? ------------------------------ In Sprint-3/implement/rotate-char.js <#207 (comment)> : > + return lowerCaseLetters[num - 2]; + } + if(char === lowerCaseLetters[i]) { + return upperCaseLetters[i + num]; + } + else if (char === upperCaseLetters[j]) { + return lowerCaseLetters[j + num]; + } + else if (char !== upperCaseLetters[j] || char !== lowerCaseLetters[i]) { + return char; + } + } + } +} + +console.log(rotateCharacter("a", 3)); //Output: "d" I wonder what your function will return when it is called with the following parameters: console.log(rotateCharacter("x", 3)); console.log(rotateCharacter("Y", 1)); You can try making your function to work on lowercase letters first, and then duplicate your code with tiny modification to apply them to uppercase letters. Also, try taking advantage of the % operator so that you don't to use any loop in your solution. ------------------------------ In Sprint-3/revise/implement/count.test.js <#207 (comment)> : > @@ -15,3 +15,10 @@ // And a character char that does not exist within the case-sensitive str, // When the function is called with these inputs, // Then it should return 0, indicating that no occurrences of the char were found in the case-sensitive str. + +const countChar = require('./count'); + +test('description', () => { + expect(countChar('aaaabfdjaa', 'a')).toBe(6); + expect(countChar('bbbbbbbbb', 'a')).toBe(0); +}); Have you tried running this test? It could help you detect a bug in your code. ------------------------------ In Sprint-3/revise/implement/get-ordinal-number.js <#207 (comment)> : > + } + else if (num % 10 === 1) { + return num + 'st'; + } + else if (num % 10 === 2) { + return num + 'nd'; + } + else if (num % 10 === 3) { + return num + 'rd'; + } + else { + return num + 'th'; + } +} + +module.exports = getOrdinalNumber; No Jest test script? ------------------------------ In Sprint-3/revise/implement/is-prime.js <#207 (comment)> : > @@ -0,0 +1,9 @@ +function isPrime(num) { + if (num <=1) return false; + for(let i = 2; i < num; i++) { You can possibly improve the performance of the code in the following manners: - Return true if num is 2 - Return false if num is an even number (we have already checked 2) - Use a loop to check if num can be fully divided by an odd number >= 3 (but <= Math.sqrt(num)) - In the loop, avoid calling Math.sqrt(num) repeatedly by first assigning the value of Math.sqrt(num) to a variable once, and then refer to the variable in the condition of the loop. ------------------------------ In Sprint-3/revise/implement/password-validator.test.js <#207 (comment)> : > @@ -14,3 +14,9 @@ To be valid, a password must: You must breakdown this problem in order to solve it. Find one test case first and get that working */ +const isValidPass = require('./password-validator'); + +test("Is prime number", function(){ Are we testing prime number ? ------------------------------ In Sprint-3/revise/implement/password-validator.test.js <#207 (comment)> : > @@ -14,3 +14,9 @@ To be valid, a password must: You must breakdown this problem in order to solve it. Find one test case first and get that working */ +const isValidPass = require('./password-validator'); + +test("Is prime number", function(){ + expect(isValidPass('Calohh35#')).toBe(true); + expect(isValidPass('alohh35#')).toBe(false); +}); These tests does not check if the function can correctly return false for each specific type of invalid passwords. For example, the test only check if the function can return false when a password does not contain any uppercase letter, but the test does not check if the function can return false when a password is missing a lowercase. Can you include more test cases to see if the function can return false for each type of invalid passwords? They could possibly help you *detect a bug* in your code. ------------------------------ In Sprint-3/revise/implement/repeat.js <#207 (comment)> : > @@ -0,0 +1,5 @@ +function repeat(str, count) { + return str.repeat(count); According to the spec, what should happen if count is a negative number? ------------------------------ In Sprint-3/implement/get-card-value.js <#207 (comment)> : > + return Number(cardValue); + } + } + else if (/^[JQK]$/.test(cardValue)) { + return 10; + } + else if (cardValue === 'A') { + return 11; + } +} + +console.assert(getCardValue('2♠') === 2, 'The given value of 2♠ returns 2'); +console.assert(getCardValue('A♠') === 11, 'The given value of "A" returns 11'); +console.assert(getCardValue('8♠') === 8, 'The given value of 8♠ returns 8'); +console.assert(getCardValue('J♠') === 10, 'The given value of J returns 10'); +console.assert(getCardValue('Q♠') === 10, 'The given value of 2♠ returns 10'); What do you expect from the following function calls? Does your function return the value you expected? getCardValue("0Q♠"); getCardValue("010♠"); getCardValue("02♠"); getCardValue("0x02♠"); getCardValue("2.1♠") ------------------------------ In Sprint-3/implement/get-card-value.js <#207 (comment)> : > + +const getCardValue = (card) => { + const cardValue = card.slice(0, -1); + + if(/^\d+$/.test(cardValue)) { + if (Number(cardValue) >= 2 && Number(cardValue) <= 10) { + return Number(cardValue); + } + } + else if (/^[JQK]$/.test(cardValue)) { + return 10; + } + else if (cardValue === 'A') { + return 11; + } +} I think you missed this description: // 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." — Reply to this email directly, view it on GitHub <#207 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BC3JQEZNEAX5PS2P2GPLGBT2EI7YPAVCNFSM6AAAAABSV6FAWCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIOBWGEYTQNJQGY> . You are receiving this because you authored the thread.Message ID: <CodeYourFuture/Module-Structuring-and-Testing-Data/pull/207/review/2486118506 @github.com>