London | May-2026-itp | Vitalii Kmit | Sprint 3 | Implement and rewrite tests by Vitalii-code · Pull Request #1463 · CodeYourFuture/Module-Structuring-and-Testing-Data

Vitalii-code · 2026-07-04T20:14:48Z

Learners, PR Template

Self checklist

I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
My changes meet the requirements of the task
I have tested my changes
My changes follow the style guide

Changelist

In this PR I worked on adding tests with plain javascript and then with jest

illicitonion

This generally looks good, but I left a few comments to look at :) Thanks!

illicitonion · 2026-07-08T17:38:14Z

-  // TODO: Implement this function
+  if (angle > 0 && angle < 90) {
+    return "Acute angle";
+  } else if (angle == 90) {


Here you're using == but JavaScript also has a === operator. Do you know the difference? Why have you chosen to use == here? When would === make more sense?

This applies in many places in your PR - please take a look at them all!

illicitonion · 2026-07-08T17:41:59Z

+    return "Right angle";
+  } else if (angle > 90 && angle < 180) {
+    return "Obtuse angle";
+  } else if (angle == 180) {


Same as above

illicitonion · 2026-07-08T17:44:34Z


 function getCardValue(card) {
-  // TODO: Implement this function
+  const lastChar = card.length - 1;


The name of this variable suggests it contains a character. But it actually contains an index of a character. Can you think of a name that could make this more clear?

illicitonion · 2026-07-08T17:45:43Z

+  }
+
+  if (!(suit === "♠" || suit === "♥" || suit === "♦" || suit === "♣")) {
+    throw new Error("Invalid suit ", suit);


It's often useful to include in an error message what the invalid data was. Imagine you're are running a big long programme, and aren't sure exactly what data is being processed - the more context we can give in errors the better.

illicitonion · 2026-07-08T17:53:40Z

+  const rank = card.slice(0, lastChar);
+  const suit = card.slice(lastChar);
+
+  if (rank === "10") {


This works, but I'm curious - why did you decide to handle 10 specially here?

What would happen if someone passed the string "10Q" to this function? What would you want to happen? What other edge cases may you want to test for in your tests?

Well because "10♠" is three characters so I decided to write a special case for it, otherwise all the others combinations should be something like this "K♠" so only two characters. It's probably not the best way to handle this but it works 🤷

It works for half of this ("Don't have to worry about 3-character strings after this line"), but it doesn't work for e.g. the "10Q" example. Can you talk through what you expect to happen if someone passes "10Q" and what your code actually does?

illicitonion · 2026-07-08T17:54:33Z

Please revert the changes to this file, they're not relevant for this exercise.

Vitalii-code · 2026-07-09T21:13:35Z

Thank you for your suggestions. I had a go at fixing the problems mentioned.

illicitonion · 2026-07-13T14:13:37Z

+  const rank = card.slice(0, lastChar);
+  const suit = card.slice(lastChar);
+
+  if (rank === "10") {


It works for half of this ("Don't have to worry about 3-character strings after this line"), but it doesn't work for e.g. the "10Q" example. Can you talk through what you expect to happen if someone passes "10Q" and what your code actually does?

illicitonion · 2026-07-13T14:13:56Z

+  if (rank == "A") {
+    return 11;
+  } else if (rank == "J" || rank == "Q" || rank == "K") {


Why the == rather than === here?

illicitonion · 2026-07-13T14:15:20Z

+  }
+
+  if (!(suit === "♠" || suit === "♥" || suit === "♦" || suit === "♣")) {
+    throw new Error(


I don't think this works like you think it does. Can you update your tests to show that the error message is what you expect it to be?

illicitonion · 2026-07-13T14:15:28Z

+  } else if (!isNaN(rank)) {
+    return Number(rank);
+  } else {
+    throw new Error("Invalid card");


This also has the same "what was wrong?" problem

Work on 1-implement-and-rewrite-tests

8a35211

Vitalii-code added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 5, 2026

illicitonion added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jul 8, 2026

illicitonion reviewed Jul 8, 2026

View reviewed changes

illicitonion added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Jul 8, 2026

Vitalii-code added 4 commits July 9, 2026 22:02

Revert testing-guide.md

bfa68d6

Change == to ===

043de8e

Change a variable to a more appropriate name

023e69f

Extend error message in 3-get-card-value.js

92027a9

Vitalii-code added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Jul 9, 2026

Vitalii-code mentioned this pull request Jul 9, 2026

May-26-ITP | Structuring and testing data | Vitalii Kmit Vitalii-code/My-Coursework-Planner#15

Open

illicitonion reviewed Jul 13, 2026

View reviewed changes

illicitonion added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jul 13, 2026

Uh oh!

Uh oh!

Conversation

Vitalii-code commented Jul 4, 2026

Learners, PR Template

Changelist

Uh oh!

illicitonion left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vitalii-code commented Jul 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants