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

NW6/Nohe-Tekelmariyam/Module-JS/week-1 #135

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

Conversation

nohetekelmariyam
Copy link

@nohetekelmariyam nohetekelmariyam commented Nov 11, 2023

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.

Copy link

@pedram-am pedram-am left a comment

Choose a reason for hiding this comment

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

Well done Nohe! I have left some comments on your code, have a look at them!

@@ -1,4 +1,5 @@
// trying to create an age variable and then reassign the value by 1

const age = 33;
var age = 33;

Choose a reason for hiding this comment

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

That's great you used var to declare the variable age 👍

@@ -1,7 +1,10 @@
const cardNumber = 4533787178994213;
const last4Digits = cardNumber.slice(-4);
let stringNumber = cardNumber.toString();
const last4Digits =stringNumber.slice(-4);

// The last4Digits variable should store the last 4 digits of cardNumber
// However, the code isn't working
// Make and explain a prediction about why the code won't work

Choose a reason for hiding this comment

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

Nice one, you could add a little explanation of your prediction as well!

const HourClockTime12 = "20:53";
const hourClockTime24 = "08:53";
//variable can not start with numer symbol

Choose a reason for hiding this comment

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

Well explained! However, the code in line 1 needs a bit more attention!

Copy link
Author

Choose a reason for hiding this comment

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

can you explain a little bit please?

week-1/exercises/count.js Show resolved Hide resolved
week-1/exercises/decimal.js Outdated Show resolved Hide resolved
@@ -4,3 +4,5 @@ let lastName = "Johnson";

// Declare a variable called initials that stores the first character of each string in upper case to form the user's initials
// Log the variable in each case
var initials= `${firstName.charAt(0).toUpperCase()} ${middleName.charAt(0).toUpperCase()}${lastName.charAt(0).toUpperCase()}`;

Choose a reason for hiding this comment

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

You have used the correct methods. Just double-check the output. Can we make it look better?

Copy link
Author

Choose a reason for hiding this comment

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

what should i change do you think? to make look better?

// Create a variable to store the ext part of the variable

Choose a reason for hiding this comment

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

Can you try to create a variable to store the ext part of the variable as well?

// Try breaking down the expression and using documentation to explain what it means
// the num variable first find random number 1 up to 100 then round the number
// and multiplay by maximum -minimum and add one finally add minimum

Choose a reason for hiding this comment

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

That is right. Also, think about the reason and logic behind doing them and how it works to get what we intended.

Choose a reason for hiding this comment

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

Perfect! 👍

Copy link
Author

@nohetekelmariyam nohetekelmariyam left a comment

Choose a reason for hiding this comment

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

add and change some code

const HourClockTime12 = "20:53";
const hourClockTime24 = "08:53";
//variable can not start with numer symbol
Copy link
Author

Choose a reason for hiding this comment

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

can you explain a little bit please?

@@ -4,3 +4,5 @@ let lastName = "Johnson";

// Declare a variable called initials that stores the first character of each string in upper case to form the user's initials
// Log the variable in each case
var initials= `${firstName.charAt(0).toUpperCase()} ${middleName.charAt(0).toUpperCase()}${lastName.charAt(0).toUpperCase()}`;
Copy link
Author

Choose a reason for hiding this comment

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

what should i change do you think? to make look better?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants