Skip to content

London | 26-ITP-Sept | Maharit Belai | Sprint 2 | Course work - #1549

Open
MaharitB wants to merge 41 commits into
CodeYourFuture:mainfrom
MaharitB:sprint-2
Open

MaharitB wants to merge 41 commits into
CodeYourFuture:mainfrom
MaharitB:sprint-2

Conversation

@MaharitB

Copy link
Copy Markdown

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

Task code

CYF-1039

Changelist

I have created a new pull request based on the instruction left on the comment section

Questions

@netlify

netlify Bot commented Sep 20, 2026

Copy link
Copy Markdown

Deploy Preview for cyf-onboarding-module ready!

Name Link
🔨 Latest commit ca1cf85
🔍 Latest deploy log https://app.netlify.com/projects/cyf-onboarding-module/deploys/6ab055ab21fe83000829ede8
😎 Deploy Preview https://deploy-preview-1549--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.

@github-actions

This comment has been minimized.

@MaharitB MaharitB added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Sep 20, 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 20, 2026
@MaharitB MaharitB changed the title london | 26-ITP-sept | Maharit Belai | Sprint-2 | Coursework london | 26-ITP-sept | Maharit Belai | Sprint2 | Course work Sep 20, 2026
@github-actions

This comment has been minimized.

@MaharitB MaharitB added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Sep 20, 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 20, 2026
@MaharitB MaharitB changed the title london | 26-ITP-sept | Maharit Belai | Sprint2 | Course work London | 26-ITP-Sept | Maharit Belai | Sprint 2 | Course work Sep 20, 2026
@MaharitB MaharitB added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Sep 20, 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 redoing the branch, it is clean now.

There are some issues to fix here:

  1. 2-mandatory-errors/1.js, 2.js and 4.js all still stop with an error when you run them. See my comment on each.

  2. 1-key-exercises/3-paths.js, see my comment.

  3. 1-key-exercises/4-random.js, see my comment.

  4. 3-mandatory-interpret/1-percentage-change.js answer a), see my comment.

  5. 3-mandatory-interpret/2-time-format.js line 1 has been changed from the original, see my comment.

  6. 3-mandatory-interpret/2-time-format.js answers b) and f), see my comment.

  7. Could you run Prettier over the Sprint-2 folder? Six of the files are not formatted at the moment.

Add the Needs Review label again when you have pushed.

@@ -2,3 +2,4 @@

const age = 33;
age = age + 1;

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 explanation on line 5 is right. Run the file though. It still stops with the same error, because line 3 is still const. This section wants the code fixed as well as explained.

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.

Done! Thank you.

@@ -3,3 +3,4 @@

console.log(`I was born in ${cityOfBirth}`);
const cityOfBirth = "Bolton";

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 explanation on line 6 is right. This file still stops when you run it though. What would you move, so that line 4 prints "I was born in Bolton"?

Comment thread Sprint-2/2-mandatory-errors/4.js Outdated
@@ -1,2 +1,4 @@
const 12HourClockTime = "8:53pm";

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.

You are right about why this happens. Both names are still here though, so node cannot read the file at all. What would you rename them to?


// https://www.google.com/search?q=slice+mdn No newline at end of file
const startDirIndex = filePath.lastIndexOf("Users");
const dir = filePath.slice(startDirIndex, lastSlashIndex);

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.

Run this file and look at the dir line. It prints Users/mitch/... but the path starts with a /, so one character is missing. Two things to think about. Which index does lastIndexOf("Users") give you? And what happens to this code if the path does not contain the word Users at all? Your ext on lines 9 and 10 is a good example of the approach that would work here.

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.

My first thought was that on the image "/ " was under the root and the dir starts from "home".
so, that is why I considered dropping the "/". But now I have fixed it.

Comment thread Sprint-2/1-key-exercises/4-random.js Outdated
//num represents a number
//lets start by (maximum-minimum +1) which the output is 100.
//math.random()*100 returns random number between 0 and 100
//math.floor()it rounds a number to thier nearest integer,

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.

What does Math.floor(4.6) give you? And what does Math.round(4.6) give you? One of them rounds to the nearest whole number and the other does something else. Also, line 6 says num represents "a number". Can you be more exact about which numbers it can be?

@MaharitB MaharitB Sep 20, 2026

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.

Math.floor(4.6) gives me the the largest integer between 4 and 4.6which is 4.
Math.round(4.6) always return a value between 0 and 1 that is decimal number. even if any value is provided for the method.

// d) Identify all the lines that are variable declarations

// e) Describe what the expression Number(carPrice.replaceAll(",","")) is doing - what is the purpose of this expression?
// There are two function calls. line 4 and 5

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.

You have found the two lines, but the question asks how many function calls there are. Line 4 has two of them, one inside the other. What is Number(...) doing there, and what is replaceAll(...) doing? Then look at line 10 as well.

@@ -1,4 +1,4 @@
const movieLength = 8784; // length of movie in seconds
const movieLength = 3000; // length of movie in seconds

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 was 8784 in the original. Trying another value is the right instinct, but put the original back. The program you are explaining should be the one you were given.

//A.There are six variable declararions

// d) Interpret line 4, what does the expression assigned to totalMinutes mean?
//B. no function calls

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.

b) says there are no function calls. Have a look at line 10. What is console.log(...) doing there?

And on f) at line 29: you say you tried various values. Try 59, then -60, then 90.5. What does each one print? Would you show a time that way?

@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 Sep 20, 2026
@abdishakoor-dev

Copy link
Copy Markdown
Contributor

Ignore point 5 on my review, about putting movieLength back to 8784. Question f) asks you to try different values, so changing that line is what it wants. Everything else on the list still stands.

@MaharitB MaharitB 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 Sep 20, 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.

2 participants