CODEWITHSUNDEEP
javascriptif-statementternary-operator
espresso_coffee
espresso_coffee

Reputation: 6110

Is there a better way to reduce multiple if statements?

I have a function that validates few different conditions. Here is the example of my function:

function checkData() {
  var errorMsg = "",
    fld1 = 0, //Number(document.getElementById('fld1').value),
    fld2 = 5, //Number(document.getElementById('fld2').value),
    fld3 = 1, //Number(document.getElementById('fld3').value),
    fld4 = 0; //Number(document.getElementById('fld4').value);

  if (!fld1) {
    errorMsg += "Error 1\n\n";
  }

  if (fld1 === fld4) {
    errorMsg += "Error 2\n\n";
  }

  if (fld2 > fld4) {
    errorMsg += "Error 3\n\n";
  }

  if (fld3 > 3) {
    errorMsg += "Error 4\n\n";
  }

  if (errorMsg !== "") {
    var check = confirm(errorMsg + "\n Do you want to submit the form?");

    if (check) {
      return true;
    } else {
      return false;
    }
  }

  return true;
}
<button onclick="checkData();">Click Here</button>

In the example above I hard coded some values for testing purpose. However, I'm wondering if I can refactor this code and find the better way of achieving the same result? Would ternary operators fit better? Or there is another way to get this to work? Thank you.

Upvotes: 0

Views: 77

Answers (3)

MarcoS
MarcoS

Reputation: 17711

In this use-case I think the 'multiple-ifs' solution is quite clear so it is the one to use.

If you want to optimize a bit, I can only suggest

        if(check){
            return true;
        }else{
            return false;
        }

to become

return !!check;

(the two exclamatives simply cast any object to a boolean vale :-))

Upvotes: 3

Alen.Toma
Alen.Toma

Reputation: 4870

The whole check variable is pointless. So return confirm is all that you need

function checkData() {
	var errorMsg = "",
		fld1 = 0, //Number(document.getElementById('fld1').value),
		fld2 = 5,//Number(document.getElementById('fld2').value),
		fld3 = 1,//Number(document.getElementById('fld3').value),
		fld4 = 0;//Number(document.getElementById('fld4').value);

	if(!fld1){
		errorMsg += "Error 1\n\n";
	}

	if(fld1 === fld4){
		errorMsg += "Error 2\n\n";
	}

	if(fld2 > fld4){
		errorMsg += "Error 3\n\n";
	}

	if(fld3 > 3){
		errorMsg += "Error 4\n\n";
	}

    return errorMsg !== "" ? confirm(errorMsg + "\n Do you want to submit the form?") : true

}
<button onclick="checkData();">Click Here</button>

Upvotes: 1

Adam
Adam

Reputation: 1754

You could refactor your if statements using the ternary operator. But chances are that it would make your code far harder to read. You could replace

if(check){
    return true;
}else{
    return false;
}

With just return check; as this is a boolean statement anyway.

Also, as far as readability goes it would be nice to label your field variables something more meaningful, as knowing that fld2 should always be greater than fld4 isn't immediately obvious from the name.

And if you don't care about highlighting the specific error codes then you could of course merge some of your checks together and just return false without the error codes specified, but I suspect you will want to keep that functionality.

Upvotes: 0

Related Questions