Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LONDON001/LOVETH-CLARA-OKAFOR/Module-Structuring-and-Testing-Data/WEEK2-/SPRINT-2 #228

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

loveth900
Copy link

@loveth900 loveth900 commented Dec 15, 2024

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.

@loveth900 loveth900 added 📅 Sprint 2 Assigned during Sprint 2 of this module Complete Participant to add when work is complete and review comments have been addressed labels Dec 15, 2024
@loveth900
Copy link
Author

Hi @cjyuan its will be a pleasure if you can help review my work, and i must say that am glad working with you cause the errors makes me know a lot of things, kind regard.

@cjyuan
Copy link

cjyuan commented Dec 15, 2024

"Completed" or "Need Review"?

@loveth900 loveth900 added Needs Review Participant to add when requesting review and removed Complete Participant to add when work is complete and review comments have been addressed labels Dec 16, 2024
@loveth900
Copy link
Author

hi @cjyuan its need review, its an error sorry for that,

Copy link

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

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

You missed some instructions in the spec.
I used "Todo" to mark the items you should follow up.

@@ -1,8 +1,19 @@
// Predict and explain first...
// Prediction : it will return "the sum of 10 and 32 is null". Because the function returns something but it's null.
Copy link

Choose a reason for hiding this comment

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

undefined is not the same as null in JS. So which value is returned from the original function?

@@ -1,4 +1,5 @@
// Predict and explain first...
// Prediction : I think the function won't return anything because it's using console.log() and not return.
Copy link

Choose a reason for hiding this comment

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

Todo

The instructions in readme.md also ask for "correcting the code".

a + b;
}

console.log(`The sum of 10 and 32 is ${sum(10, 32)}`);
Copy link

Choose a reason for hiding this comment

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

Todo

The current script still does not show the sum as 42. You also need to correct the code in the function.

function capitalise(str) {
let str = `${str[0].toUpperCase()}${str.slice(1)}`;
str = `${str[0].toUpperCase()}${str.slice(1)}`; // No `let`, just reassign
Copy link

Choose a reason for hiding this comment

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

Alternatively, you can declare a new variable or just return the value of the expression (without storing the value in a variable first).

{ input: "00:01", expected: "12:01 am" },
{ input: "12:01", expected: "12:01 pm" },
{ input: "13:00", expected: "1:00 pm" },
];
Copy link

Choose a reason for hiding this comment

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

Todo

You also need fix the function if any test fails.

@@ -4,3 +4,13 @@
// You will need to declare a function called toPounds with an appropriately named parameter.

// You should call this function a number of times to check it works for different inputs


function toPounds(kilograms) {
Copy link

Choose a reason for hiding this comment

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

Todo

If you refer to interpret/to-pounds.js in Sprint 1, you will find the code there is to convert pence string in the form "399p" to British pound "£3.99". This function should be based on those code.

@@ -29,3 +29,6 @@ function formatTimeDisplay(seconds) {
// d) What is the value assigned to num when pad is called for the last time in this program? Explain your answer

// e) What is the return value assigned to num when pad is called for the last time in this program? Explain your answer

Copy link

Choose a reason for hiding this comment

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

Todo

In this exercise, you need to answer questions (a)-(e) by tracing the execution of the code (that is, figure out in what order every statement and every function call is executed) .

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review labels Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Reviewed Volunteer to add when completing a review 📅 Sprint 2 Assigned during Sprint 2 of this module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants