1
AddPatient = {};

AddPatient.Firstname = FirstNameValue || PatientModel.errorMsg('FirstName',FirstNameValue);
AddPatient.LastName = LastNameValue || PatientModel.errorMsg('LastName',LastNameValue);

AddPatient is an Object and i am checking it whether its blank or not before sending the request.

PatientModel.js

errorMsg: function(title,FirstNameValue,LastNameValue) {
        if(FirstNameValue === undefined || FirstNameValue === ' ' && LastNameValue === undefined || LastNameValue = ' ') {
          alert('FirstName and LastName are missing');
          return false;
          } else {
          alert(+title 'is missing');
                        return false; 
          } 
        }

I have a form, where i have FirstName and LastName field and i have to check it should not be blank. I want a single function in javascript which can work.

Is this the right way to do it?

2 Answers 2

2

I can see a couple of problems in your code.

  • Mismatch between errorMsg()'s expected arguments and how it is called
  • Syntax error in second alert()
  • Bad expression inside if statement

Mismatch between errorMsg()'s expected arguments and how it is called

Your errorMsg() function expects three arguments, but you only pass it two at a time:

errorMsg: function(title,FirstNameValue,LastNameValue) {
    ....
}

... and then ....

.errorMsg('FirstName',FirstNameValue);
.errorMsg('FirstName',LastNameValue);

If you really want to use both values inside errorMsg(), you need to pass them both every time, in the same order the function expects them:

PatientModel.errorMsg('FirstName',FirstNameValue,LastNameValue);
PatientModel.errorMsg('LastName',FirstNameValue,LastNameValue);
// This is weird code, but it'll work

Syntax error in second alert()

This is simple enough to fix, and could have been just a typo.

alert(+title 'is missing');
      ^      ^_There's something missing here.
      |_This will only try to convert title to a number

What you want is this:

alert(title + 'is missing');

Bad expression inside if statement

if(FirstNameValue === undefined || FirstNameValue === ' ' && LastNameValue === undefined || LastNameValue = ' ') {

This won't work as you expect, because && has greater precedence than ||, meaning the expression will be evaluated as such:

if (
     FirstNameValue === undefined
 || (FirstNameValue === ' ' && LastNameValue === undefined)
 ||  LastNameValue = ' '
) {

You would need parenthesis to fix the precedence:

if( (FirstNameValue === undefined || FirstNameValue === ' ') && (LastNameValue === undefined || LastNameValue = ' ') ) {

This is irrelevant, actually, because the expression can be simplified like this:

// If these values are taken from a form input, they are guaranteed to be strings.
if(FirstNameValue.length === 0 && LastNameValue.length === 0) {

Or even better, like this:

// Uses regular expressions to checks if string is not whitespace only
var WHITESPACE = /^\s*$/;
if( WHITESPACE.test(FirstNameValue) && WHITESPACE.test(FirstNameValue)){

How I would fix your code

This would be an incomplete answer if I didn't provide a correct version of your code, so here it goes. Notice that I separate filling-in of information and its validation in two steps.

PatientModel.js :

validate: function(patient){
    var WHITESPACE = /^\s*$/;
    var errors = [];

    if( WHITESPACE.test(patient.FirstName) ){
        // No first name
        if( WHITESPACE.test(patient.LastName) ){
            // No last name either
            errors.push('FirstName and LastName are missing');
        }
        else {
            // Only first name missing
            errors.push('FirstName is missing');
        }
    }
    else if( WHITESPACE.test( patient.LastName) ){
        // Only last name missing
        errors.push('LastName is missing');
    }

    // Returns array of errors
    return errors;
}



Your other code:

AddPatient = {};
AddPatient.Firstname = FirstNameValue; 
AddPatient.LastName = LastNameValue;

errors = PatientModel.validate(AddPatient);
if( errors.length != 0 ){
    alert('You have the following errors:\n' + errors.join('\n'));
}

Edit: a different, perhaps better, approach. The only difference is we now write validate() as a method of a Patient object:

>>> PatientModel.js:

var WHITESPACE = /^\s*$/;

// Creates a new empty Patient
function Patient(){
    this.FirstName = '';
    this.LastName = '';
}

// Adds a validate() method to all Patient instances
Patient.prototype.validate: function(){
    var errors = [];

    if( WHITESPACE.test(this.FirstName) ){
        // No first name
        if( WHITESPACE.test(this.LastName) ){
            // No last name either
            errors.push('FirstName and LastName are missing');
        }
        else {
            // Only first name missing
            errors.push('FirstName is missing');
        }
    }
    else if( WHITESPACE.test( thisLastName) ){
        // Only last name missing
        errors.push('LastName is missing');
    }

    // Returns array of errors
    return errors;
}




>>> Your other code :

patient = new Patient();
patient.FirstName = FirstNameValue;
patient.LastName = LastNameValue;

errors = patient.validate();
if( errors.length != 0 ){
    alert('You have the following errors:\n' + errors.join('\n'));
}
Sign up to request clarification or add additional context in comments.

1 Comment

@John Yes, I should have written test instead of match. Sorry about that. I've edited the answer.
1

I would recommend using the Jquery validate plugin for this functionality. It will let you do a hole range of clientside validation. Including required validation.

If you just want pure javascript then your method is fine. I'd make it a more generic validate method which would check each validation rule you have and if it fails add the message to an array. After all your checks are finished if the count of the array is grater then zero then output complete list of errors.

2 Comments

@John Cooper You can do it without Jquery. See edit to answer (any reason why you don't want to use it>)
No, his method is not fine. The expression in the if won't work as expected because of the relative precedence between && and ||, and there is a syntax error in the second alert.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.