London | May-2025 | Mohamed Ibrahim | Sprint-2 #544

sidicamoli · 2025-06-14T10:48:40Z

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

Changelist

In this PR, the necessary changes made and questions answered for Sprint-2 will be presented..
This request encompasses a range of enhancements and the integration of numerous functionalities associated with JavaScript fundamentals

Questions

None for now, thanks!

a-robson · 2025-07-01T19:19:07Z

Sprint-2/2-mandatory-debug/0.js

+  return a * b;
+}
+
+console.log(`The result of multiplying 10 and 32 is ${multiply(10, 32)}`);


What happens when you run this code now? Why doesn't the faulty code give the same error as before?

When I run this code now, it works correctly and logs:
"The result of multiplying 10 and 32 is 320."
The error doesn’t happen anymore because the multiply function is now properly defined with (return a * b;) statement. Before, it probably didn’t return a value, which caused undefined in the output.

When it is run now the output is

The result of multiplying 10 and 32 is 320
The result of multiplying 10 and 32 is 320

Why does the output appear twice? And why it is that although you haven't amended the original code the 'undefined' error has disappeared?

Because the multiplication function or code is being called or executed two times, causing the same output to print twice. I think because the runtime was reset or refreshed, clearing previous errors or incomplete states that caused the 'undefined' error earlier.

So how can you change the code in the file so that the multiply function doesn't get called twice?

I removed the first version of the multiply function, which only logged the result. Now, there is only one version of the function in the file that uses the "return" command, so the output only appears once.

Yes. Generally you will want to comment out or remove the initial, faulty code example in these exercises.

a-robson · 2025-07-01T19:20:31Z

Sprint-2/3-mandatory-implement/1-bmi.js

+// Example test cases
+console.log(calculateBMI(70, 1.73)); // Expected: 23.4
+console.log(calculateBMI(80, 1.8));  // Expected: 24.7
+console.log(calculateBMI(50, 1.6));  // Expected: 19.5


Your implementation is good and I like that you tried a few different inputs. However, what is the output when you run this code?

Thank you!
When I run the code, the output is:
23.4
24.7
19.5
These match the expected BMI values, so the function is working as intended.

When I run node 1-bmi.js I get
undefined
undefined
undefined
Can you see why?

The undefined output happens because the BMI function didn’t have a return statement. Just like in the earlier multiply example, without return, the function gives undefined when logged. Adding return fixes it.

How many times is the calculateBMI function defined in the file?

The calculateBMI function was defined more than once in the file, which is likely why it was undefined earlier. I’ve now removed the extra definitions, leaving only one function that returns the correct result.

I'm afraid I'm still seeing multiple definitions of the function. Are you sure you committed your changes? If we can sort this out we can move this review to 'Complete'

Thanks for the feedback! I think it's sorted out now.

Yeah, I see the changes now. Good job overall. I'll mark this review as complete.

Thank you so much! I appreciate your feedback and help throughout the review process. I'm glad it's all good now!

a-robson · 2025-07-01T19:20:58Z

Sprint-2/3-mandatory-implement/2-cases.js

+
+// Example usage:
+console.log(toUpperSnakeCase("hello there"));           // "HELLO_THERE"
+console.log(toUpperSnakeCase("lord of the rings"));     // "LORD_OF_THE_RINGS"


Great job. Code is concise and correct. But is 'input' a good variable name? What might be an alternative?

Thank you!
You're right, I think the input is quite generic. So, a better alternative could be something more descriptive like text, phrase, or sentence to better reflect the expected value. For example:
function toUpperSnakeCase(phrase) {
return phrase.toUpperCase().replace(/ /g, "_");
}
This makes the purpose of the variable clearer.

Yes, 'phrase' is a much better variable name.

a-robson · 2025-07-01T19:21:31Z

Sprint-2/3-mandatory-implement/3-to-pounds.js

+// The code "console.log(toPounds("399p"));" shows the price of £3.99.
+// The code "console.log(toPounds("50p"));" shows that the amount is £0.50.
+// The console.log function displays the value "4p" (which is equivalent to £0.04).
+// The code "console.log(toPounds("1234p"));" shows the price of £12.34.


Really nice implementation. However you should add some tests.

Thanks! I’ll definitely add some proper tests to make it more clear.

Thanks! I added some tests.

a-robson · 2025-07-01T19:22:19Z

Sprint-2/5-stretch-extend/format-time.js

+console.assert(
+  formatAs12HourClock("23:59") === "11:59 pm",
+  "Failed: 23:59 should be 11:59 pm"
+);


I'd expect 03:22 to be converted to 3:22 am. What would your program return? Another thing to think about it is what would happen if we had invalid input e.g. 25:00?

Thanks for the feedback!
I’ve updated the function to return "3:22 am" (no leading zero) and added validation for invalid input like "25:00".

a-robson

Overall this is really good. I've made a few comments - let me know if you want me to expand or discuss.

…lace

a-robson · 2025-07-02T14:10:51Z

Sprint-2/5-stretch-extend/format-time.js

 function formatAs12HourClock(time) {
-  const hours = Number(time.slice(0, 2));
-  if (hours > 12) {
-    return `${hours - 12}:00 pm`;
+  const [hourStr, minuteStr] = time.split(":");
+
+  const hours = Number(hourStr);
+  const minutes = Number(minuteStr);
+
+  // Validate input
+
+  if (
+    isNaN(hours) || isNaN(minutes) ||
+    hourStr.length !== 2 || minuteStr.length !== 2 ||
+    hours < 0 || hours > 23 ||
+    minutes < 0 || minutes > 59
+  ) {
+    return "Invalid time";
  }
-  return `${time} am`;
-}

-const currentOutput = formatAs12HourClock("08:00");
-const targetOutput = "08:00 am";
-console.assert(
-  currentOutput === targetOutput,
-  `current output: ${currentOutput}, target output: ${targetOutput}`
-);
+  let displayHours = hours % 12 || 12;
+  const period = hours >= 12 ? "pm" : "am";
+
+  return `${displayHours}:${minuteStr} ${period}`;
+}


Great implementation. It's quite different from the original and that did make me wonder whether you might have relied a bit too much on ChatGPT or some other AI. If you did use AI my advice is to use it as little as possible at this stage in your development as a programmer. AI can fool you into thinking you've learned something when you're not actually absorbing or retaining information.

Thanks for the feedback! I understand your point and take it seriously. I’ve mainly been using AI for guidance to help me understand how to improve or fix my own code, not to copy solutions. Moving forward, I’ll rely more on my own thinking and use AI as a learning tool, not a crutch.

…ry-debug/0.js

…turn

sidicamoli added 15 commits June 14, 2025 11:23

made a change

5139db4

Key Errors | Fixed variable scope and redeclaration issue in 1.js

d50dbf4

Key Errors | Fixed variable redeclaration issue in 0.js

1e36a83

Key Errors | Fixed syntax error in 2.js by using a valid parameter name

83d4a0c

Key Errors | Fixed variable redeclaration issue in 0.js

13c3193

Key Errors | Editing in 2.js by using a valid parameter name

dbe07fb

Debug | Fixed multiply function to return value instead of logging

8233a1e

Debug | Fixed return statement in sum function

6245319

Debug | Fixed getLastDigit function to use input parameter

b84aa41

Implement | Completed 1-bmi.js with calculation and explanation

44ca700

Implement | Added toUpperSnakeCase function with examples

e1d14d0

Implement | Completed UPPER_SNAKE_CASE conversion function

02af9b7

Implement | Created reusable toPounds function

0e31007

Interpret | Completed time-format questions with full answers

7016082

Implement | Extended time formatting functionality

9fe0bfd

sidicamoli added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jun 15, 2025

a-robson 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 1, 2025

a-robson reviewed Jul 1, 2025

View reviewed changes

a-robson 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 1, 2025

sidicamoli added 3 commits July 1, 2025 23:50

Add tests for toPounds function

b1fcc25

Implement BMI calculator that returns result rounded to one decimal p…

1ba20be

…lace

Update time format and handle invalid input

e270252

sidicamoli added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 2, 2025

a-robson reviewed Jul 2, 2025

View reviewed changes

sidicamoli added 3 commits July 2, 2025 17:47

Fix multiply function to return value instead of just logging.mandato…

846337a

…ry-debug/0.js

removed duplicate calculateBMI functions and kept one with correct re…

59293ec

…turn

Cleaned up duplicate multiply function and kept only the correct version

144d15f

sidicamoli mentioned this pull request Jul 7, 2025

London | May-2025 | Structuring and Testing Data | Mohamed Ibrahim sidicamoli/My-Coursework-Planner#13

Open

illicitonion closed this Sep 1, 2025

Uh oh!

London | May-2025 | Mohamed Ibrahim | Sprint-2 #544

London | May-2025 | Mohamed Ibrahim | Sprint-2 #544

Uh oh!

Conversation

sidicamoli commented Jun 14, 2025

Learners, PR Template

Changelist

Questions

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!

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!

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!

a-robson 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants