Skip to content

Conversation

@eyuell21
Copy link

@eyuell21 eyuell21 commented Feb 6, 2025

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.
Completed the exercises given for sprint two.

Questions

Ask any questions you have for your reviewer.

@eyuell21 eyuell21 added 📅 Sprint 2 Assigned during Sprint 2 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Feb 6, 2025

// =============> write your explanation here

// trying to redeclare the variable called decimalNumber twice in the function.

Choose a reason for hiding this comment

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

also- the code was not invoking the convertToPercentage function which you corrected

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

// =============> write your explanation here
// =============> write your explanation here: The sum of 10 and 32 is undefined because a+b is written under the return statement.

Choose a reason for hiding this comment

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

it is actually because that is what the function returns. return; will always return undefined. It is the same as writing return undefined

Copy link
Author

Choose a reason for hiding this comment

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

Noted. Thanks.

// console.log(`The last digit of 806 is ${getLastDigit(806)}`);

function getLastDigit() {
// Now run the code and compare the output to your prediction

Choose a reason for hiding this comment

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

You've corrected the mistake but your explanation isn't quite right. The problem was the number wasn't passed into the function as a parameter

// You will need to come up with an appropriate name for the function
// 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

Choose a reason for hiding this comment

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

fun fact- this is sometimes called screaming snake case!

let pounds = paddedPenceNumberString.substring(0,paddedPenceNumberString.length - 2);
const pence = paddedPenceNumberString.substring(paddedPenceNumberString.length - 2).padEnd(2, "0");
return `£${pounds}.${pence}`
}

Choose a reason for hiding this comment

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

Great job so far !

try and test this one here:

function changeToPounds(penceString){
return £${(parseInt(penceString) / 100).toFixed(2)};
}

This is a robuster method to convert to pounds, and easier to understand . You don't need to worry about where de letter p will appear in the string, when using parseInt , it removes all letters elements from the value and takes just the numbers.

And then , just divide it to 100 to transform pence in pounds .

Copy link
Author

Choose a reason for hiding this comment

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

Done. This method is very simple and amazing. Thankyou!

Copy link

@fernandamelov fernandamelov left a comment

Choose a reason for hiding this comment

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

Great job so far !

try and test this one here for 3-mandatory-implement/3-to-pounds.js:

function changeToPounds(penceString){
return £${(parseInt(penceString) / 100).toFixed(2)};
}

This is a robuster method to convert to pounds, and easier to understand . You don't need to worry about where de letter p will appear in the string, when using parseInt , it removes all letters elements from the value and takes just the numbers.

And then , just divide it to 100 to transform pence in pounds .

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

function changeToPounds(penceString){
let penceStringWithoutTrailingP = penceString.substring(0,penceString.length - 1);

Choose a reason for hiding this comment

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

string.slice() is a little easier for this

let penceStringWithoutTrailingP = penceString.slice(0, -1);

Copy link
Author

Choose a reason for hiding this comment

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

Okay. Thankyou!

// a) When formatTimeDisplay is called how many times will pad be called?
// =============> write your answer here

// three times

Choose a reason for hiding this comment

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

is there a way you can check this?

Copy link
Author

Choose a reason for hiding this comment

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

We can use a global variable, which is increased each time the function is called.(got it from stackoverflow).

var myFuncCalls = 0;

function pad(num) {
myFuncCalls++;
console.log( "I have been called " + myFuncCalls + " times" );
return num.toString().padStart(2, "0");

}

// three times
// Call formatTimeDisplay with an input of 61, now answer the following:

// b) What is the value assigned to num when pad is called for the first time?

Choose a reason for hiding this comment

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

this depends on the number passed in at the start. If it's big enough it will be more than 0

Copy link
Author

Choose a reason for hiding this comment

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

Noted, Thankyou for the review.


function getAngleType(angle) {
if (angle === 90) return "Right angle";
if (angle === 90) return "Right angle";

Choose a reason for hiding this comment

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

none of these if statements need to be else if or else because you are returning a value from the function. Whenever you return something from a function, the execution stops so none of the code below is executed.

Also, you know that an angle is < 90 because the code carried on past that if statment so you only need to do:

if(angle < 90) return "Acute angle";
if(angle < 180) return "Obtuse angle";
if(angle < 360) return "Reflex angle";

etc

Feel free to message me if this doesn't make sense

Copy link
Author

Choose a reason for hiding this comment

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

This commit and the one after this were not supposed to be on this Pull request as they are from sprint three they are here by accident. I'll have to redo them with other the sprint 3 question on a new branch and a new PR for sprint 3.

else if (angle === 180) return "Straight angle"
else if (angle > 180 && angle < 360) return "Reflex angle"
else if(angle === 360) return "Full angle"
else return "please insert a valid angle (0-356)"

Choose a reason for hiding this comment

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

angles up to 360 should be valid


function isProperFraction(numerator, denominator) {
if (numerator < denominator) return true;
if (Math.abs(numerator) < Math.abs(denominator) && numerator !=0 && denominator != 0) return true;

Choose a reason for hiding this comment

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

Careful. the Math.abs() method just takes any negative number and turns it into a positive which would break this function in certain scenarios.

For example: console.log(isProperFraction(-5, 3)); would incorrectly return false when it is a proper fraction.

Do you know how you could fix this bug? Feel free to message me if you need a hand

Copy link

@BenSymons BenSymons left a comment

Choose a reason for hiding this comment

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

A few suggestions and some small mistakes here and there but for the most part, well done!

@eyuell21 eyuell21 requested a review from BenSymons February 15, 2025 23:14
@halilibrahimcelik halilibrahimcelik added Reviewed Volunteer to add when completing a review with trainee action still to take. Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Feb 17, 2025
@halilibrahimcelik
Copy link

Well done you can close this PR now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Reviewed Volunteer to add when completing a review with trainee action still to take. 📅 Sprint 2 Assigned during Sprint 2 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants