Skip to content

London | 26-ITP-Sep | Mahir Shah | Sprint 3 | Sprint 3 Coursework - #1625

Open
MahirShah300 wants to merge 18 commits into
CodeYourFuture:mainfrom
MahirShah300:coursework/sprint-3
Open

MahirShah300 wants to merge 18 commits into
CodeYourFuture:mainfrom
MahirShah300:coursework/sprint-3

Conversation

@MahirShah300

@MahirShah300 MahirShah300 commented Sep 28, 2026 •

Copy link
Copy Markdown

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

Task code

CYF-1053

Changelist

Answered the questions, fixed all bugs

@netlify

netlify Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for cyf-onboarding-module ready!

Name Link
🔨 Latest commit 9b453d8
🔍 Latest deploy log https://app.netlify.com/projects/cyf-onboarding-module/deploys/6ac0fd2b9335f200086bcf99
😎 Deploy Preview https://deploy-preview-1625--cyf-onboarding-module.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
2 paths audited
Performance: 100 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 86 (no change from production)
PWA: -
View the detailed breakdown and full score reports
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@MahirShah300 MahirShah300 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Sep 28, 2026
@github-actions

This comment has been minimized.

@github-actions github-actions Bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Sep 28, 2026
@github-actions

This comment has been minimized.

@MahirShah300 MahirShah300 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Sep 30, 2026
@madhuranakate madhuranakate 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 Sep 30, 2026
Comment thread Sprint-3/1-key-errors/0.js Outdated
// =============> write your new code here

function capitalise(str) {
str = `${str[0].toUpperCase()}${str.slice(1)}`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A general rule of thumb is - if you overwrite the value of parameter, it could cause bugs down the code as the original value is lost.

could you perhaps create a local variable instead and return that ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a change to use a variable capitaliseStr

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. str keeps its first value now.

Comment thread Sprint-3/1-key-errors/2.js Outdated
// =============> write your new code here


function square(num = 3) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will default the number to 3 if not passed (which is not what the requirement was). Instead, Can you try calling the square with different numbers and see the output

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the default parameter, it squares the number when calling it with different numbers

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. square works for any number now.

// Use the MDN string documentation to help you find a solution
// This might help https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/toUpperCase
function toUpperSnakeCase(inputString) {
return inputString.toUpperCase().replaceAll(" ", "_");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was the replaceAll used ? does this print exactly what is expected or does it add some additional strings which were not part of input string ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replaceAll goes through a string and removes all cases of the first input, and replaces it with the second input. It prints what is expected, and the only thing added are _ in places of " ". If there's any unexpected whitespace " " after a string, they would replaced with _

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clear answer, thanks.

}
function formatAs12HourClock(time) {
const hours = Number(time.slice(0, 2));
if (

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good effort. could you think of other scenarios you might have missed ?

  1. what happens when you pass empty strings - formatAs12HourClock(" :30")

  2. what happens when you pass nothing - formatAs12HourClock()

  3. what happens when you pass without semicolon - formatAs12HourClock(1230)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.Empty strings are caught and show "Not a valid time"
2. This would cause errors. I fixed it by using default parameter ""
3. Without a semicolon is also caught. However if the input is not a string it would cause errors, which I fixed by checking if the input is type string

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the stretch part, so it does not block Complete. But one check: what does formatAs12HourClock(" :30") give now? I get "12:30 am".

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does give "12:30 am". This is because there is validation on whitespace in the input string. I have added it now

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed now. Good.

isNaN(minutes) ||
hours >= 24 ||
hours < 0 ||
minutes >= 60 ||

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are not checking if he hours and minutes are integers eg. "12:30.5" would pass your tests

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"12:30.5" actually doesn't pass the tests because of my checks for only one "." and one ":", but I added a check to make sure the numbers are integers

@madhuranakate madhuranakate 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 Sep 30, 2026
@MahirShah300 MahirShah300 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 Oct 1, 2026

@abdishakoor-dev abdishakoor-dev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes. Your answers are clear and correct. In time-format.js e) you explain each step of the while loop. That is very good.

One thing before I can mark this Complete:

  1. 3-to-pounds.js: see my comment on line 24.

Add the Needs Review label again once you've pushed.

.substring(paddedPenceNumberString.length - 2)
.padEnd(2, "0");

console.log(`£${pounds}.${pence}`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your function prints the price, but it does not give it back. What does toPounds("399p") return?

You explained this yourself in 2-mandatory-debug/0.js: a function that only uses console.log gives back undefined.

Say another part of a program needs the price. For example, it wants to add it to a sentence. Can it get the price from toPounds now? What would you change?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not currently. I have changed the console.log to return £${pounds}.${pence} instead

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your function returns the price now, which is right. And you call it a few times with different values. But how do you know it works each time? Line 6 says to call it "to check it works for different inputs".

There are a few ways to do this:

  • Wrap each function call in lines 27-31 in console.log, so the returned value is printed in the terminal: console.log(toPounds("10p"));
  • Or use console.assert, like you did in format-time.js, to compare each result with the price you expect.

@abdishakoor-dev abdishakoor-dev 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 Oct 1, 2026
@MahirShah300 MahirShah300 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 Oct 2, 2026

@abdishakoor-dev abdishakoor-dev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. toPounds gives the price back now, which is the important part.

Two small things before I can mark this Complete, both in 3-to-pounds.js:

  1. Line 24: see my reply there.
  2. The file fails Prettier since your last change. Open it, right click, choose Format Document, save, and push.

Add the Needs Review label again once you've pushed.

@abdishakoor-dev abdishakoor-dev 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 Oct 3, 2026
@MahirShah300 MahirShah300 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 Oct 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants