CYF ITP South Africa - Cape Town | Emmanuel Siziba | Module-Structuring-and-Testing-Data | Week 3 #217

EmmanuelSiziba · 2024-12-04T20:19:04Z

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

I left some comments in your code.

If you have any questions, feel free to ask on Slack or respond here.

cjyuan · 2024-12-04T20:40:56Z

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

+      return "Right angle";
+    }
+
+    if(angle  < 90){


Technically, 0 < acute angle < 90. Since the spec didn't specify, we can assume angle is always a positive number.
You don't have to change your code.

okay so you are saying since an acute angle is any angle that measures form 0 to 90, I should leave my solution as it is .

I was saying you can leave your solution as it is because the spec didn't say if the function needs to deal with angles that are negatives or angles that are more than or equal to 360.

I have put in the else statement with the corresponding test case

cjyuan · 2024-12-04T20:43:25Z

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

+    if(angle < 360){
+      return "Reflex angle" 
+    }
+  }


If it were up to you, what do think the function should return when angle is >= 360?

It will give us an angle number equal to or greater than 360.

Honestly, I was confused here because I just couldn't figure a way to code this statement " angle is greater than 180 degrees and less than 360 degrees"

so I thought I would just focus on the last part that says less than 360 degrees but I see that was only half of the answer please check I fixed my if statement.

Your previous code is correct because the previous if statements have eliminated all the cases where angle is less than or equal to 180. That is, if angle is less than or equal to 180, one of the return statements would have been executed.

What I was asking is that, what if angle is more than 360? For example, 400. Someone could call your function as console.log( getAngleType(400) ).

I have put in the else statement with the corresponding test case

cjyuan · 2024-12-04T20:52:45Z

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

+
+
+function getCardValue(cardString) {
+    const rank = cardString[0];


Since the rank can possibly be any value, we cannot assume it contains only one character.
So the function should also expect values like 'AA♠', '1000♠', and '2.1♠' and consider them as invalid card rank.

Can you modify your code to report these as invalid card ranks?

I have modified it, did I do it the right way?

Now your function have syntax error. Have you tested your code after you modified it?

Your approach is still not correct; if you just check the first character in the rank, you won't be able to distinguish between "2♠'" and "22♠'"

Since there are so many possible invalid ranks, but there are only 13 valid ranks, you should focus on checking if the rank in the is given cardString is exactly one of these 13 valid ranks. Any other ranks would be invalid.

I have fixed it and now it can handle a invalid ranks

cjyuan · 2024-12-04T20:54:33Z

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

+
+
+test('getCardValue returns 10 for J, Q or K', () => {
+    expect(getCardValue('J, Q, K')).toBe(10);


The spec means

expect(getCardValue('J♠')).toBe(10); expect(getCardValue('Q♠')).toBe(10); expect(getCardValue('K♠')).toBe(10);

Thank you for this,i have edited my test case

cjyuan · 2024-12-04T20:55:33Z

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

+
+
+test('getCardValue returns parseInt(rank) for 2 & 9', () => {
+  expect(getCardValue('2')).toBe(parseInt('2'));


Need to always include a suite character (or any character) as the last character in the "card string".

Also, "10" is a valid card rank.

is this what you are saying i must do :
test('getCardValue returns parseInt(rank) for 2 to 10', () => {
expect(getCardValue('2♣')).toBe(2);
expect(getCardValue('3♥')).toBe(3);
expect(getCardValue('4♦')).toBe(4);
expect(getCardValue('5♠')).toBe(5);
expect(getCardValue('6♣')).toBe(6);
expect(getCardValue('7♥')).toBe(7);
expect(getCardValue('8♦')).toBe(8);
expect(getCardValue('9♠')).toBe(9);
expect(getCardValue('10♣')).toBe(10);
});

The function only needs to check the rank value and can assume the last character is a valid suite character.
So that means you can also set up tests like:

expect(getCardValue('2#')).toBe(2); expect(getCardValue('9#')).toBe(9); expect(getCardValue('10#')).toBe(10); expect(getCardValue('Q#')).toBe(10); expect(getCardValue('K#')).toBe(10); expect(() => getCardValue('200♠')).toThrow('Invalid card rank.'); expect(() => getCardValue('2.1♠')).toThrow('Invalid card rank.');

I have fixed it

cjyuan · 2024-12-04T21:47:06Z

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

+});
+
+test('invalidates  weak password', () => {
+    const password = 'weak'; 


The objective is not to test weak password, but to test if the function can correctly return true when the given password is valid, and return false when the given password is invalid.

For example, can your function return false when the given password is missing only lowercase letters?

cjyuan · 2024-12-04T21:52:03Z

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

+});
+
+//test('throws an error for negative count', () => {
+//    expect((repeat) => repeat('hello', -1)).toThrow('Count cannot be negative');


This line should be
expect(() => repeat('hello', -1)).toThrow('Count cannot be negative');

cjyuan · 2024-12-04T21:55:09Z

Sprint-3/revise/implement/repeat.js

@@ -0,0 +1,13 @@
+function repeat(str, count){
+    if (count < 0) {
+        throw new Error("Count cannot be nagative");


Typo; the string value should be "'Count cannot be negative'"

cjyuan · 2024-12-04T21:58:30Z

Sprint-3/revise/investigate/find.js

@@ -21,5 +21,11 @@ console.log(find("code your future", "z"));

 // a) How the index variable updates during the call to find
 // b) What is the if statement used to check
+
+//The if statement is used to determine or check if the string has the character that we are looking for 


if (str[index] === char) does not deal with the whole string but part of the string.

May I suggest you feed the code to ChatGPT and see how else the code can be explained? From time to time we can learn some new terms or more concise way to describe code.

okay noted, I am doing that.
I understand what you saying and I will feed it so I can learn more. So its safe to say it is very important is to learn to describe code in both simple and technical terms concisely.

cjyuan · 2024-12-04T22:01:14Z

Sprint-3/revise/investigate/find.js

 // c) Why is index++ being used?
+
+// I think it is used to locate the exact location of the character we will be looking for in our string.


The code is designed to locate the given character in the given string, but the explanation you gave does not quite answer the questoin.

Can you also try asking ChatGPT the same question (after you feed it the code) so see how else one can answer this question?

Spelling error

fixing my test cases

cjyuan

Some files still have bugs or not yet fully implemented.

cjyuan · 2024-12-09T01:48:40Z

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

+    if(angle < 360){
+      return "Reflex angle" 
+    }
+  }


Your previous code is correct because the previous if statements have eliminated all the cases where angle is less than or equal to 180. That is, if angle is less than or equal to 180, one of the return statements would have been executed.

What I was asking is that, what if angle is more than 360? For example, 400. Someone could call your function as console.log( getAngleType(400) ).

cjyuan · 2024-12-09T01:59:49Z

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

+      return "Right angle";
+    }
+
+    if(angle  < 90){


I was saying you can leave your solution as it is because the spec didn't say if the function needs to deal with angles that are negatives or angles that are more than or equal to 360.

cjyuan · 2024-12-09T02:08:43Z

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

+
+
+function getCardValue(cardString) {
+    const rank = cardString[0];


Now your function have syntax error. Have you tested your code after you modified it?

Your approach is still not correct; if you just check the first character in the rank, you won't be able to distinguish between "2♠'" and "22♠'"

Since there are so many possible invalid ranks, but there are only 13 valid ranks, you should focus on checking if the rank in the is given cardString is exactly one of these 13 valid ranks. Any other ranks would be invalid.

cjyuan · 2024-12-09T02:15:08Z

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

+
+
+test('getCardValue returns parseInt(rank) for 2 & 9', () => {
+  expect(getCardValue('2')).toBe(parseInt('2'));


The function only needs to check the rank value and can assume the last character is a valid suite character.
So that means you can also set up tests like:

expect(getCardValue('2#')).toBe(2); expect(getCardValue('9#')).toBe(9); expect(getCardValue('10#')).toBe(10); expect(getCardValue('Q#')).toBe(10); expect(getCardValue('K#')).toBe(10); expect(() => getCardValue('200♠')).toThrow('Invalid card rank.'); expect(() => getCardValue('2.1♠')).toThrow('Invalid card rank.');

cjyuan · 2024-12-09T02:17:51Z

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

+});
+
+test('getCardValue throws error for invalid rank', () => {
+  expect(() => getCardValue('invalid rank')).toThrow('Invalid card rank.');


Well, at least those invalid cases that I suggested.

cjyuan · 2024-12-12T21:33:48Z

Sprint-3/revise/implement/get-ordinal-number.test.js

+});
+
+test('converts 43 to an ordinal number', () => {
+  expect(getOrdinalNumber(43)).toBe('43rd');


Could group similar test in the following manner:

test("Expect 'st' as suffix when the number is 1", () => { expect(getOrdinalNumber(1)).toBe('1st'); }); test("Expect 'st' as suffix when last digit is 1 but last two digits are not 11, 12, or 13", () => { expect(getOrdinalNumber(21)).toBe('21st'); expect(getOrdinalNumber(131)).toBe('131st'); expect(getOrdinalNumber(1001)).toBe('1001st'); }); ...

cjyuan · 2024-12-12T21:34:24Z

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

+  if (num === 2) return true;
+
+  const sqrtNum = Math.sqrt(num);
+  for (let i = 3; i <= sqrtNum; i += 2) {


This is good.

cjyuan · 2024-12-12T21:35:38Z

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

+    if (!/[!#$%.*&]/.test(password)) {
+      return false;
+    }
+


Looks good. How do you plan to check if the password is one of the passwords already in used?

You can assume these "used passwords" are stored in some array.

cjyuan · 2024-12-12T21:36:41Z

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

+const checkPassword = require('./password-validator');
+
+test('checks if a password is valid', () => {
+    const password = 'valid!';


"valid!" is not a valid password according to the specified rules.

cjyuan · 2024-12-12T21:38:39Z

package.json

+  "description": "Like learning a musical instrument, programming requires daily practice.",
+  "main": "index.js",
+  "dependencies": {
+    "ansi-escapes": "^4.3.2",


Why do you need all these dependency packages for? "jest" would be enough.

EmmanuelSiziba added 30 commits December 3, 2024 16:00

installed jest

0455567

get angle type function

a5805ac

exporting angle type function for testing

49b57b0

test case file

23eb70b

5 type of angle tests

6408987

get card value function

ed96256

epsorting the getcardvalue function

9cc7315

file for get-card-value test cases

d8f1af0

getCardValue test cases

babde67

is properFraction function

7b610f9

exporting isproperfraction function

aab6e2e

file for is proper function test case

968ddf8

isproperfraction test cases

0537d6f

isValidTriangle function

0e9c76e

exporting the isvalidtriangle function

699b87f

isvalid triangle test case

cc6526e

commenting

11783cb

rotate character function

4fbab41

exporting the rotateCharacter function

099c1aa

rotate character test cases

2a4f69a

rotate character file and test cases

487fb5c

create a count js file

9e9ac0c

count character function

66706d0

exporting the count char function

909cc91

get ordinal number js file

83d1fb8

getOrdinalNumber function

bbac933

get ornial number test cases

3da5a95

exporting the get ordinal number function

6cab33c

js file for is prime function

1e17560

js file for password validator project

de9797e

card validator function

1c7e88b

EmmanuelSiziba added the Needs Review Participant to add when requesting review label Dec 4, 2024

EmmanuelSiziba closed this Dec 4, 2024

cjyuan reviewed Dec 4, 2024

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review labels Dec 4, 2024

EmmanuelSiziba reopened this Dec 8, 2024

EmmanuelSiziba added 13 commits December 8, 2024 17:05

Update get-angle-type.js

9c2bed0

Update get-card-value.js

5618c98

Update get-card-value.test.js

e1fd233

Update get-card-value.test.js

8a32c3f

Update repeat.js

660da3e

Spelling error

Update repeat.test.js

2d225ca

Update find.js

dcf6ce9

Update find.js

3e307a3

Update password-validator.test.js

5e6d56f

Update is-prime.test.js

3603ef8

Update is-prime.js

2f4f386

Update is-prime.js

293e59f

Update get-ordinal-number.test.js

9830f5b

fixing my test cases

cjyuan reviewed Dec 12, 2024

View reviewed changes

EmmanuelSiziba added 7 commits December 14, 2024 12:44

Update get-angle-type.js

e94ff41

Update get-angle-type.js

7dc7129

Update get-card-value.js

5d6a07d

Update is-proper-fraction.js

806bfcd

Update is-proper-fraction.test.js

70d037e

Update rotate-char.js

8877358

Update get-ordinal-number.test.js

5d38bd1

EmmanuelSiziba 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 17, 2024



		function getCardValue(cardString) {
		const rank = cardString[0];



		test('getCardValue returns 10 for J, Q or K', () => {
		expect(getCardValue('J, Q, K')).toBe(10);



		test('getCardValue returns parseInt(rank) for 2 & 9', () => {
		expect(getCardValue('2')).toBe(parseInt('2'));

		// c) Why is index++ being used?

		// I think it is used to locate the exact location of the character we will be looking for in our string.

CYF ITP South Africa - Cape Town | Emmanuel Siziba | Module-Structuring-and-Testing-Data | Week 3 #217

Are you sure you want to change the base?

CYF ITP South Africa - Cape Town | Emmanuel Siziba | Module-Structuring-and-Testing-Data | Week 3 #217

Conversation

EmmanuelSiziba commented Dec 4, 2024

Learners, PR Template

Changelist

Questions

cjyuan left a 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

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

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

cjyuan left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment