Javascript javascript1 week4/parisa #111
Conversation
|
In general, the code for the individual tasks looks pretty good - you've done a good job extracting relevant data from strings. But there's perhaps a bit of confusion around the overall code structure. You might want to take some of the top-level code where you set all your variables and move it inside of the conditionals later in the function. This will ensure it runs only when it needs to, and you don't accidentally overwrite information provided by the user. You may also want to move out some of the variables to global scope (i.e. outside the function). That way they will be available when the function runs multiple times! |
|
|
||
| function getReply(comand){ | ||
| // name | ||
| let sentence = 'Hello, my name is Benjamin.'; |
There was a problem hiding this comment.
Everything in this function that's not inside of a conditional if(...) will be run every time the function is called. So even if the user only wants to check the time, then we still run everything up to line 43, even though it's not really needed!
There was a problem hiding this comment.
they are moved.
| // name | ||
| let sentence = 'Hello, my name is Benjamin.'; | ||
| let parts = sentence.split(" "); | ||
| let name = parts.slice(4).join(" "); |
There was a problem hiding this comment.
Good job parsing the name from the input string, but since we're reading from a hard-coded string - Hello, my name is Benjamin. on line 4, then the value of the name variable will be set to Benjamin every time the function runs.
If the user instead types, Hello, my name is Stephen., then we would probably expect the name variable to be set to Stephen instead.
In order for this to work, you might want to declare the name variable outside of the getReply() function. That way it will be available to the function if it's called again a second time. So the user can say Hello, my name is Stephen. and then when they later ask What is my name? then the assistant will reply with the correct answer, instead of being reset to Benjamin.
There was a problem hiding this comment.
It moved outside the function:)
| let list=[]; | ||
| let toDoList='Add fishing to my todo'; | ||
| let parts2 = toDoList.split(" "); | ||
| let toDo = parts2.slice(1,2).join(" "); |
There was a problem hiding this comment.
This works well if it's just a single word that we need to extract, but what happens if it's more than one? For example, what happens if I say Add going out to dinner to my todo?
There was a problem hiding this comment.
I used -3 instead of 2.
| // Time | ||
| let currentTime = new Date().toLocaleTimeString(); | ||
|
|
||
| if (comand === 'Hello, my name is Benjamin.') { |
There was a problem hiding this comment.
Is there a way we can change this to handle different names? Maybe we could instead check if the string starts with Hello, my name is, so that it works for any name?
There was a problem hiding this comment.
I used command.startwith :)
| @@ -0,0 +1,94 @@ | |||
|
|
|||
| function getReply(comand){ | |||
There was a problem hiding this comment.
Minor: command should have two 'm' characters 😄
There was a problem hiding this comment.
done:)
|
Hi Jordan, Thank you for your feedback. I now understand that I should write general code for handling general commands, and I realized this only after submitting the assignment. Your comments have been very helpful. First, I moved the list and myName variables outside of the function to ensure they persist across multiple calls. Next, I reorganized the if clauses into conditional statements to make the code more efficient, so JavaScript doesn’t need to check the entire codebase for every command. Finally, I replaced direct command comparisons with command.startsWith to improve the flexibility of the code. Thank you once again for your time and valuable insights. I truly appreciate your help! Best regards, |
|
Hey Parisa, That looks fantastic after the changes, great job! |
|
Thanks a lot.
…On Tue, 10 Dec 2024 at 21.39, Jordan McFarlane ***@***.***> wrote:
***@***.**** approved this pull request.
—
Reply to this email directly, view it on GitHub
<#111 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BKUMTYQZ6ZNBWGXGXSJOSVL2E5GO5AVCNFSM6AAAAABS7X7HFKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIOJTGU2DEOJUHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***
com>
|
No description provided.