Javascript javascript1 week4/yuusuf #124
Conversation
… one that gets the name.
…on and the two first commands are working now.
|
Overall looks pretty good. I like your general handling of edge cases. In most places that things can go wrong handling user input you've got an There's a little inconsistency going on with the formatting, but it's not big stuff. Consider running a formatting tool from your code editor to fix up whitespace and other minor formatting issues. |
| let month = monthNames[day.getMonth()]; | ||
| let year = day.getFullYear(); | ||
|
|
||
| return `${today} ${month} ${year}` |
There was a problem hiding this comment.
There's a neat (newish) method in Javascript called .toLocaleString() that will format a date based on some options you provide to it. It could be easier than doing the formatting this way.
For example:
let date = new Date();
let dateFormatOptions = {
month: "long",
year: "numeric"
};
return date.toLocaleString("en", dateFormatOptions);
I'll let you figure out the rest 😉
| function calcMath(command){ | ||
| let num1 = parseFloat(command.split(" ")[2]); | ||
| let operator = command.split(" ")[3]; | ||
| let num2 = parseFloat(command.split(" ")[4]); |
There was a problem hiding this comment.
We're calling command.split(" ") three times here on these three lines. While that's not a big problem for short strings, in general we want our programs to do the least work they can get away with. That helps us write fast performant code that doesn't frustrate users, and it helps us save the planet a little at a time by using less computing power too!
So consider calling command.split(" ") just once and assigning the result to a variable. You can then use that variable on each of these lines.
| case "/": | ||
| return num1 / num2; | ||
| default: | ||
| return "Im not able to solve that."; |
There was a problem hiding this comment.
Good stuff having a fallback case if none of the others match. Consider adding some alternative or follow-up action for the user to help them understand how they can solve the error on their own.
For example: I'm not able to solve that. Try again using a +, -, *, or / operator in your equation
|
|
||
|
|
||
| function setTimer(command) { | ||
| let minutes = parseInt(command.split(" ")[4]); |
There was a problem hiding this comment.
How could you change this method if the user wanted to set a timer for say... 30 seconds?
|
|
||
|
|
||
| function addTodoList(command){ | ||
| let todo = command.slice(4, command.indexOf("to my todo")); |
There was a problem hiding this comment.
It's not something super important to handle, but it's always good to think about edge cases and how our code might behave.
For example, what if someone wanted to add think of some things to add to my todo to their todo list? Their command would be:
add think of some things to add to my todo to my todo
Which is a little silly for sure! But it would be nice if our code handled it.
What the code will currently do is see the first instance of to my todo when we call command.indexOf("to my todo"). That means it will assign think of some things to add to the todo variable, whereas it would ideally assign think of some things to add to my todo instead.
We can fix this by using a slightly different method from indexOf(). Fortunately Javascript has a lastIndexOf() method for exactly this reason. Instead of returning the index of the first matching string, it will return the index of the last one!
Again - not a super important fix for now. The lesson is mostly just to keep thinking about our code and how it might break, so that we can make deliberate choices about whether we fix it!
|
|
||
|
|
||
| function getReply(command){ | ||
| if(command.startsWith("Hi my name is")){ |
There was a problem hiding this comment.
What happens if the user types in this command using different casing? E.g. ALL UPPERCASE? Would be nice to have our code just handle that scenario!
No description provided.