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

Movie Chatbot #296

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

Movie Chatbot #296

wants to merge 11 commits into from

Conversation

Fannyhenriques
Copy link

Copy link
Contributor

@HIPPIEKICK HIPPIEKICK left a comment

Choose a reason for hiding this comment

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

Great job on completing this project! 🎉 Let's have a look at some feedback.

JavaScript

  • Your functions are well structured! Just be mindful of the order when using arrow functions. Have a look at last week's Zoom Recordings page.
  • Good usage of conditionals ⭐

Clean Code

  • Try to be consistent with your function naming, i.e. they should be named after what they are doing. Also, they should be named using camelCase (starting with lowercase letter)
  • Be consistent with whether you're using double or single quotes

Keep up the good work! 💯

Comment on lines +183 to +186
inputWrapper.style.display = 'flex';
inputWrapper.style.width = '100%';
inputWrapper.style.boxSizing = 'border-box';
inputWrapper.style.padding = '10px';
Copy link
Contributor

Choose a reason for hiding this comment

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

This could maybe be refactored 👀

@Anna2024WebDev
Copy link

Anna2024WebDev commented Sep 22, 2024

What a fun and nice chatbot you have created @Fannyhenriques! 🎬 An extra plus for the end reply as well "Grab your popcorn!" 🍿😄

The chat bot functions works well and it is easy to follow the flow. I also find your code easy to read.

Suggestions of improvement:

As Matilda already mentioned, be consistent with using camelCase for your functions. (lowercaseUppercase) Example below;

image

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