Have completed the first homework#3
Conversation
IsmaelMartinez
left a comment
IsmaelMartinez
left a comment
There was a problem hiding this comment.
In general, pretty good job. Minor comments added.
| } | ||
|
|
||
| // Making changes when clicking orange button.*/ | ||
| var orangeButton = document.querySelector('#orangeBtn'); |
There was a problem hiding this comment.
I prefer to move the var declarations to the top of the file or function.
There was a problem hiding this comment.
same goes for greenButton, submitButton... etc.
|
|
||
| // Setting the form is Valid by using boolean.(true) | ||
| var formIsValid = true; | ||
| /* if the namebox with value.length is invalid, changed background color of box to red |
There was a problem hiding this comment.
Long comments tend to indicate that a part of the code can be replaced by an inner function.
The nameboxMv if statement can be replaced by a function like "isNameValid" returning true or false depending on the case.
You can apply the same for email and description. This reduces this function complexity and removes the need of comments.
I will elaborate more in the next comment.
|
|
||
| /* if the form is valid, alert (" thanks "), change namebox, describeYourself | ||
| and email's value to empty string value.*/ | ||
| if (formIsValid) { |
There was a problem hiding this comment.
If we replace each function for a boolean check function, then we can replace this code for:
if (isNameValid() && isEmailValid() && isDescribeYourselfValid()) {
The implementation can still be the same a part from formIsValid = false that is not needed anymore.
The reason of doing so is to make it easier to expand other validations and to read and maintain the code.
There was a problem hiding this comment.
Thank you so much for your time to check the code and giving very useful advices.. :) :) I will edit them.. :) 👍
| } | ||
| } | ||
| } | ||
| var url = "http://ajax-cyf.eu-west-1.elasticbeanstalk.com/chatroom/?id=c"; //server location |
|
|
||
|
|
||
| // This works. Posting, Sending (params) to Server | ||
| var request = new XMLHttpRequest(); //creating a request object |
There was a problem hiding this comment.
By creating an object twice, only the last implementation will be use. (1st one will be discarded)
Make sure that is what you want, otherwise use the same request object or create a 'postRequest, getRequest'
| request.onreadystatechange = function () { | ||
| if (request.readyState === 4) { // check if a response was sent back | ||
| if (request.status === 200) { // check if request was successful | ||
| document.querySelector(".display-3").innerHTML = JSON.parse(request.responseText)['message']; |
There was a problem hiding this comment.
Try to follow always the same patter. If you are assiging the querySelector to a variable, do it across all your code
3 participants
Hello mentors,
Good evening. I hope you are fine. Could you please check if my codes are ok? Thank you so much for all your help.
Kind Regards,
Won