-
-
Notifications
You must be signed in to change notification settings - Fork 218
London|25-ITP-September|Alexandru Pocovnicu|Sprint 1| Coursework #719
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
base: main
Are you sure you want to change the base?
London|25-ITP-September|Alexandru Pocovnicu|Sprint 1| Coursework #719
Conversation
…ressions in comments
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
5 similar comments
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start on this sprint's tasks, I have spotted a few areas where you could improve code further, or things you might want to discuss
|
||
const dir = ; | ||
const ext = ; | ||
const dir = filePath.slice(1,lastSlashIndex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a directory, the first / is important to include when programming. Can you make sure it is captured?
|
||
// https://www.google.com/search?q=get+first+character+of+string+mdn | ||
|
||
// I really can't understand any of the documentation from developer.mozilla |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is difficult to understand about the MDN docs?
const age = 33; | ||
age = age + 1; | ||
//the "age" variable has been declared with a const and already assigned a value and it can not | ||
// be reassigned another value because of the "const" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What change is needed to fix this?
|
||
console.log(`I was born in ${cityOfBirth}`); | ||
const cityOfBirth = "Bolton"; | ||
//a variable needs to be declared before being initialized |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change the code so that it works?
//total time in minutes without the remaining seconds | ||
|
||
// e) What do you think the variable result represents? Can you think of a better name for this variable? | ||
//"Movie length" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it clear what the difference is between the new "movielength" and original movieLength variables? Could I understand the difference by quickly reading these two variable names?
//"Movie length" | ||
|
||
// f) Try experimenting with different values of movieLength. Will this code work for all values of movieLength? Explain your answer | ||
//it will work with all values as long as they are numbers, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider all the edge cases - are there any inputs that might give strange formatting?
If you want to investigate this, it might be helpful to try running the code yourself, but changing this (e.g. removing some parts) to see what, if anything, stops working. |
Learners, PR Template
Self checklist
Changelist
Completed Sprint 1 exercises in which i added comments, fixed errors,and interpreted code
Questions
In 1-key-exercises - 4-random.js I understand the methods but don't understand why this " (maximum - minimum + 1)) + minimum " is necessary s