Sahil Dhiman
Sahil Dhiman

Reputation: 55

How can I optimize my validation code in reactjs?

Below, I have mentioned my JS code. So, Kindly suggest or guide me that there is any better approach to implement the validation on form submit for react project or is it right approach which i have implemented already?

submitUserForm = (e) => {        
    e.preventDefault();        
    const { formError } = this.state;
    let valid = true;
    if(document.getElementById('useremail').value === "") {
        valid = false;
        formError.email = "Kindly enter your email id"            
    }
    else {
        valid = true;
        formError.email = ""            
    }
    if(document.getElementById('userpassword').value === "") {
        valid = false;
        formError.password = "Kindly enter the password"           
    }
    else {
        valid = true;
        formError.password = ""            
    }         
    if(valid === true) {        
        this.formSubmitApi();         
    }     
    this.setState({
        isValid: valid,
        formError
    })       
}

Upvotes: 0

Views: 76

Answers (3)

Alberto Perez
Alberto Perez

Reputation: 2922

First of all, in order to improve your code, you need to have controlled components (Inputs in this case) in order to store your values in the state and then use them in the submitUserForm function, also, instead of valid variable, I'll use a validation function like (Also, make the useremail and userpassword objects, in order to store the errors there):

state = {
    useremail: { value: '', error: '' },
    userpassword: { value: '', error: '' },
};

validateValue = value => {
    return value !== undefined && value !== null && value !== '';
};

submitUserForm = e => {
    const { useremail, userpassword } = this.state;

    e.preventDefault();

    // If both the 'useremail' and the 'userpassword' pass the 'validateValue' validations, then send the data
    if (this.validateValue(useremail.value) && this.validateValue(userpassword.value)) {
        this.formSubmitApi(useremail.value, userpassword.value);

        // After sending the info, we reset our two states to initial state
        this.setState({useremail: { value: '', error: '' }, userpassword: { value: '', error: '' } });
    } else {
        // If 'useremail' don't pass the validation we store the error
        if (!this.validateValue(useremail.value)) {
            this.setState({ useremail: { value: useremail.value, error: 'Please enter a valid email' }});
        }

        // If 'userpassword' don't pass the validation we store the error
        if (!this.validateValue(userpassword.value)) {
            this.setState({ userpassword: { value: userpassword.value, error: 'Please enter a valid password' }});
        } 
    }
};

I think this is a more clean approach, you only have to states instead of three as before and you can get all the information that you need from those states.

Upvotes: 1

narayansharma91
narayansharma91

Reputation: 2353

This is how you can improve your code:

    submitUserForm = (e) => {        
            e.preventDefault();        
            const formError = {}
           if(document.getElementById('useremail').value === "") {
                formError.email = "Kindly enter your email id"            
           }
          if(document.getElementById('userpassword').value === "") {
               formError.password = "Kindly enter the password"           
           }
          Object.keys(formError).length === 0 && this.formSubmitApi(); 
           this.setState({
             formError
            })       
        }

Upvotes: 2

Domino987
Domino987

Reputation: 8774

I would change 2 things:

  1. Make the inputs controlled, so that you have better and direct control of the input. Accessing the inputs with document.getElementById is bad practice. Save the text of each input within the state or use refs.
  2. I would change the check for valid input into something like this:

    const errors = {};

    errors.email = "Kindly enter your email id"

    if (Object.keys(errors).length === 0) {...}

It should be a new object, so that you don't mutate the state and this makes it easier because you don't have a second variable.

I hope this helps. Happy coding.

Upvotes: 0

Related Questions