WM4 | Fatima Safana | Module-Structuring-and-Testing-Data | WEEK6 #235

Fatimasfn · 2024-12-21T21:03:53Z

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.

-test files -implemented function get-angle-type.js

-tested the isProperFraction

cjyuan

I left some comments and suggestions in your code.

cjyuan · 2024-12-22T15:43:10Z

Sprint-3/implement/get-angle-type.js

+function getAngleType(angle) {
+    if (angle === 90) {
+        return "Right angle"
+    } else if ( angle < 90) {


The spec does not give an exact specification of acute angle.
By definition, an acute angle cannot be less than or equal to 0.

cjyuan · 2024-12-22T15:46:43Z

Sprint-3/implement/get-card-value.js

+function getCardValue(card) {
+   const singleDigitRank = Number(card.substring(0,1));
+   // const doubleDigitRank = Number(card.substring(0,2));
+
+        if (card.startsWith("10")){
+            return 10;
+        } else if (singleDigitRank >= 2 && singleDigitRank <=9){
+            return singleDigitRank;
+        } else if (card.startsWith("J") || card.startsWith("Q") || card.startsWith("K")){
+            return 10;
+        } else if (card.startsWith("A")){
+            return 11
+        } else return "Invalid Cards";
+
+}


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

getCardValue("99Q♠"); getCardValue("100♠"); getCardValue("2K♠"); getCardValue("KK♠"); getCardValue("AA♠")

cjyuan · 2024-12-22T15:48:31Z

Sprint-3/implement/implement-sprint3.test.js

+test('Given numerator and denominator, return the type of fraction', () => {
+   expect(isProperFraction(2, 3)).toBe(true); 
+   expect(isProperFraction(5, 3)).toBe(false);
+   expect(isProperFraction(2, 0)).toBe("error");
+});


Why not test also cases where numerator and/or denominator are negative values?

cjyuan · 2024-12-22T15:49:16Z

Sprint-3/implement/implement-sprint3.test.js

+test('Given an angle in degrees, return the type of angle', () => {
+  expect(getAngleType(90)).toBe("Right angle");
+  expect(getAngleType(45)).toBe("Acute angle");
+  expect(getAngleType(160)).toBe("Obtuse angle");
+  expect(getAngleType(180)).toBe("Straight angle");
+  expect(getAngleType(300)).toBe("Reflex angle");
+});


Could also test invalid cases.

cjyuan · 2024-12-22T15:50:49Z

Sprint-3/implement/implement-sprint3.test.js

+const getAngleType = require('./get-angle-type.js');
+const isProperFraction = require('./is-proper-fraction.js');
+const isValidTriangle = require('./is-valid-triangle.js');
+const getCardValue = require('./get-card-value.js');
+
+
+
+test('Given an angle in degrees, return the type of angle', () => {


It is better to use separate .test.js file for each unit test.

cjyuan · 2024-12-22T16:00:08Z

Sprint-3/revise/implement/is-prime.test.js

+        return true 
+    } else if ((num > 2)) {
+        let squareRootNum = Math.sqrt(num)
+        for (let i = 2 ; i <= squareRootNum; i++ ) {


Can further speed up the performance in the following manner:

return false if num is an even number

check only odd divisors in the loop

cjyuan · 2024-12-22T16:04:58Z

Sprint-3/revise/implement/password-validator.test.js

+    if (password.length >= 5){
+        return true 
+    } else if (password.length < 5) 


if (condition) { } else if (! condition) { // Opposite condition }

can be simplified as

if (condition) { } else { }

cjyuan · 2024-12-22T16:27:44Z

Sprint-3/revise/implement/password-validator.test.js

+test('if password is a valid password.', () => {
+  expect(isValidPassword("abcd")).toBe(false);     
+  expect(isValidPassword("Abc!")).toBe(false);    
+  expect(isValidPassword("ABCD1")).toBe(false);    
+  expect(isValidPassword("Dog!")).toBe(false);


These tests are not comprehensive, and they could not indicate exactly why the function fails. A better way to prepare the tests are in the following manner:

test('Check password without an uppercase letter', () => { expect(isValidPassword("1a!123")).toBe(false); expect(isValidPassword("za123!")).toBe(false); }); test('Check passwords without a digit', () => { expect(isValidPassword("!!aABC")).toBe(false); expect(isValidPassword("Abaaa!")).toBe(false); }); ...

A comprehensive test can help you detect bugs in isValidPassword().

cjyuan · 2024-12-22T16:31:45Z

Sprint-3/revise/implement/repeat.test.js

+    } return "";
+}
+
+console.log(repeat("cat", 3));


There isn't any Jest test specified in this file.

What do you expect from the following function calls?

repeat("", -1) repeat("count cant be negative", 1)

cjyuan · 2024-12-22T16:34:57Z

Sprint-3/revise/investigate/find.js

 // d) What is the condition index < str.length used for?
+//the condition checks all indexes are checked. the check keeps iterating until it finds a matching value or index is less than str length


Your answer to question (d) seems like a description of what the while loop is doing, and not specifically why the loop condition is specified as index < str.length.

Fatimasfn · 2024-12-25T21:36:10Z

I left some comments and suggestions in your code.

Thank you for reviewing, I will implement the suggestions.

Fatimasfn added 10 commits December 16, 2024 17:29

-added node modules

486d3d4

-test files -implemented function get-angle-type.js

renamed test files

9b87c50

implement is-proper-fraction

f2deeaf

-tested the isProperFraction

implemented is valid triangle function, plus tests for the function

72ddd33

implemented and tested getCardValue

859ab24

implemented and tested count.js

8895465

isPrime function implemented

0de99a5

implemented function isvalidpassword with tests

7135b39

implemented repeat()

83adeca

find.js

511097e

Fatimasfn added the Needs Review Participant to add when requesting review label Dec 21, 2024

cjyuan added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Dec 22, 2024

cjyuan reviewed Dec 22, 2024

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Dec 22, 2024

Fatimasfn added 7 commits December 26, 2024 09:51

added spec and implemented it to acute angle definition

af80044

re-implemented to pass suggested tests

891a000

added negative test cases

6c83ddf

fixed funtion,

efc544d

added more test cases

e3d3cab

added test cases to repeat()

2246997

answers to (d)

114fc6b

Fatimasfn added Complete Participant to add when work is complete and review comments have been addressed and removed Reviewed Volunteer to add when completing a review labels Dec 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WM4 | Fatima Safana | Module-Structuring-and-Testing-Data | WEEK6 #235

WM4 | Fatima Safana | Module-Structuring-and-Testing-Data | WEEK6 #235

Fatimasfn commented Dec 21, 2024

cjyuan left a comment

cjyuan Dec 22, 2024 •

edited

Loading

cjyuan Dec 22, 2024 •

edited

Loading

cjyuan Dec 22, 2024

cjyuan Dec 22, 2024

cjyuan Dec 22, 2024

cjyuan Dec 22, 2024

cjyuan Dec 22, 2024

cjyuan Dec 22, 2024

cjyuan Dec 22, 2024

cjyuan Dec 22, 2024

Fatimasfn commented Dec 25, 2024

		// d) What is the condition index < str.length used for?
		//the condition checks all indexes are checked. the check keeps iterating until it finds a matching value or index is less than str length

WM4 | Fatima Safana | Module-Structuring-and-Testing-Data | WEEK6 #235

Are you sure you want to change the base?

WM4 | Fatima Safana | Module-Structuring-and-Testing-Data | WEEK6 #235

Conversation

Fatimasfn commented Dec 21, 2024

Learners, PR Template

Changelist

Questions

cjyuan left a comment

Choose a reason for hiding this comment

cjyuan Dec 22, 2024 • edited Loading

Choose a reason for hiding this comment

cjyuan Dec 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fatimasfn commented Dec 25, 2024

cjyuan Dec 22, 2024 •

edited

Loading

cjyuan Dec 22, 2024 •

edited

Loading