add homework

[najmeh25]

No description provided.

[jmf-umbraco]

Looks pretty good! I've left a few suggestions of things that can be improved slightly, but the bulk of the code works well.

One overall change you could consider is to pull out some of the code into helper methods. The getReply() function is doing quite a lot of work now - it's determining which command the user entered, extracting relevant info from that command, and then putting together a response. You could perhaps consider dividing up the code such that each different type of command is handled by a different function. This would help it all be a bit more readable at-a-glance, and would help control the size of the getReply() function if we ever enhanced the code to support many more commands!

[jmf-umbraco]

First of all, this approach works, which is the most important part of coding!

But it's a little unorthodox - the purpose of the .split() method on strings is to split them up into many different parts. What we actually are using it for is extracting one small part of a string, but there are some other methods that are just a little better suited for different reasons - they might perform faster, or be slightly easier for other developers to read and understand.

Consider using .subscring() or .slice() instead of .split()

[jmf-umbraco]

What if the user doesn't use a starting capital? It would be nice if our program continued to work!

[jmf-umbraco]

We may want to consider using a slightly different condition instead of whether the command includes some text.

Consider what happens if the user enters the following command (and forget about casing for the moment):

Remove add things to my todo from my todo

Now this is a bit of a silly, contrived example, but the intent of the user is to remove add things to my todo from their todo list. On the other hand, our code is going to try and handle this as though the user wants to add something to their todo list, because it contains add and to my todo.

We can deal with this problem by being a bit more specific - perhaps we check that the command starts with add, not just that it contains that. Likewise we can check it ends with to my todo!

[jmf-umbraco]

Great job handling this edge case!

[jmf-umbraco]

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 😉

[jmf-umbraco]

Nice edge case handling!

[jmf-umbraco]

Great error message. We frequently are guilty of writing error messages that say things like "Invalid input", or some variation on "I can't do that". This can be frustrating to the user - because from their perspective they have no idea what's actually wrong, or what else they can try.

Your error message instead helps the user to understand how the program does work and gives clues about what the user can do about the error. It's much more helpful!

[jmf-umbraco]

Once again, this works, but there might be some easier ways of extracting the number from the string. Can you think of any?

[najmeh25]

Hi ,thank you very much for guidance and help

…

On Tue, 10 Dec 2024 at 13:09, Jordan McFarlane ***@***.***> wrote: ***@***.**** requested changes on this pull request. Looks pretty good! I've left a few suggestions of things that can be improved slightly, but the bulk of the code works well. One overall change you could consider is to pull out some of the code into helper methods. The getReply() function is doing quite a lot of work now - it's determining which command the user entered, extracting relevant info from that command, and then putting together a response. You could perhaps consider dividing up the code such that each different type of command is handled by a different function. This would help it all be a bit more readable at-a-glance, and would help control the size of the getReply() function if we ever enhanced the code to support many more commands! ------------------------------ In javascript/javascript1/week4/weeek4.js <#118 (comment)> : > @@ -0,0 +1,139 @@ + +let name = ""; +let todos=[]; + +function getReply(command) { + + if (command.includes("Hello my name is")) { + let newName = command.split("Hello my name is ")[1]; First of all, this approach works, which is the most important part of coding! But it's a little unorthodox - the purpose of the .split() method on strings is to split them up into many different parts. What we actually are using it for is extracting one small part of a string, but there are some other methods that are just a little better suited for different reasons - they might perform faster, or be slightly easier for other developers to read and understand. Consider using .subscring() or .slice() instead of .split() ------------------------------ In javascript/javascript1/week4/weeek4.js <#118 (comment)> : > @@ -0,0 +1,139 @@ + +let name = ""; +let todos=[]; + +function getReply(command) { + + if (command.includes("Hello my name is")) { What if the user doesn't use a starting capital? It would be nice if our program continued to work! ------------------------------ In javascript/javascript1/week4/weeek4.js <#118 (comment)> : > + return `You've already introduced yourself as ${name}`; + } + name = newName; + return `Nice to meet you ${name}`; + } + + + if (command.includes("What is my name")) { + if (name) { + return `Your name is ${name}`; + } else { + return "I don't know your name yet."; + } + } + +if (command.includes("Add") && command.includes("to my todo")) { We may want to consider using a slightly different condition instead of whether the command includes some text. Consider what happens if the user enters the following command (and forget about casing for the moment): Remove add things to my todo from my todo Now this is a bit of a silly, contrived example, but the intent of the user is to *remove* add things to my todo from their todo list. On the other hand, our code is going to try and handle this as though the user wants to *add* something to their todo list, because it contains add and to my todo. We can deal with this problem by being a bit more specific - perhaps we check that the command *starts with* add, not just that it contains that. Likewise we can check it *ends with* to my todo! ------------------------------ In javascript/javascript1/week4/weeek4.js <#118 (comment)> : > + return `Your name is ${name}`; + } else { + return "I don't know your name yet."; + } + } + +if (command.includes("Add") && command.includes("to my todo")) { + let todoItem = command.split("Add ")[1].split(" to my todo")[0]; + todos.push(todoItem); + return `${todoItem} added to your todo`; +} + +if (command.includes("Remove") && command.includes("from my todo")) { + let todoItem = command.split("Remove ")[1].split(" from my todo")[0]; + let index = todos.indexOf(todoItem); + if (index > -1) { Great job handling this edge case! ------------------------------ In javascript/javascript1/week4/weeek4.js <#118 (comment)> : > + return "Your todo list is empty"; + } else { + return `You have ${todos.length} todos: ${todos.join(", ")}`; + } + } + + if (command.includes("What day is it today")) { + let today = new Date(); + let day = today.getDate(); + let monthNames = [ + "January", "February", "March", "April", "May", "June", + "July", "August", "September", "October", "November", "December" + ]; + let month = monthNames[today.getMonth()]; + let year = today.getFullYear(); + return `${day}. of ${month} ${year}`; 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 😉 ------------------------------ In javascript/javascript1/week4/weeek4.js <#118 (comment)> : > +if (command.includes("What is")) { + let mathExpression = command.replace("What is", "").trim(); + let parts = mathExpression.split(" "); + let num1 = parseFloat(parts[0]); + let operator = parts[1]; + let num2 = parseFloat(parts[2]); + + + if (operator === "+") { + return num1 + num2; + } else if (operator === "-") { + return num1 - num2; + } else if (operator === "*") { + return num1 * num2; + } else if (operator === "/") { + return num2 !== 0 ? num1 / num2 : "Cannot divide by zero"; Nice edge case handling! ------------------------------ In javascript/javascript1/week4/weeek4.js <#118 (comment)> : > + let parts = mathExpression.split(" "); + let num1 = parseFloat(parts[0]); + let operator = parts[1]; + let num2 = parseFloat(parts[2]); + + + if (operator === "+") { + return num1 + num2; + } else if (operator === "-") { + return num1 - num2; + } else if (operator === "*") { + return num1 * num2; + } else if (operator === "/") { + return num2 !== 0 ? num1 / num2 : "Cannot divide by zero"; + } else { + return "I can only perform +, -, *, and / operations"; Great error message. We frequently are guilty of writing error messages that say things like "Invalid input", or some variation on "I can't do that". This can be frustrating to the user - because from their perspective they have no idea what's actually wrong, or what else they can try. Your error message instead helps the user to understand how the program *does* work and gives clues about what the user can do about the error. It's much more helpful! ------------------------------ In javascript/javascript1/week4/weeek4.js <#118 (comment)> : > + } else if (operator === "*") { + return num1 * num2; + } else if (operator === "/") { + return num2 !== 0 ? num1 / num2 : "Cannot divide by zero"; + } else { + return "I can only perform +, -, *, and / operations"; + } + } + + + if (command.includes("Set a timer for") && command.includes("minutes")) { + let timePart = command + .replace("Set a timer for", "") + .replace("minutes", "") + .trim(); + let minutes = Number(timePart); Once again, this works, but there *might* be some easier ways of extracting the number from the string. Can you think of any? — Reply to this email directly, view it on GitHub <#118 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BKXJ4FA54SE2NJSQTWA2IOD2E5J6ZAVCNFSM6AAAAABTDHC3NGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIOJTGU3DONJXGE> . You are receiving this because you authored the thread.Message ID: ***@***.*** com>