IF Statement Logic

**johnnyi** · 06-21-2005, 01:57 PM

I'm trying to write a JavaScript to assign a value to a textbox (TQScore) based on the selection from a dropdown (TQResult). For example, if the user selects "PM" from the dropdown, I want the textbox to display a "2". Whats happening is that, regardless of selection, when the function is executed, the dropdown itself is being set to "FE" and the textbox is being assigned the value of 5.
I tried to copy the value of the dropdown into a hidden textbox, and perform the script based on the value of that textbox, but it just overwrites the textbox with "FE" and assigns a score of 5.
What it appears to be doing in the if statement is setting the TQResult value to "FE" - ignoring the if statement and just performing the action (document.forms[0].TQResult.value = "FE").
How can I set this up to be a true if statement: whereas IF the value of the TQResult dropdown is equal to "PM" (for example), then set the TQScore value to a 2... and so on...

Code:

	if (document.forms[0].TQResult.value = "DNM")
		{
			document.forms[0].TQScore.value = "1";
		}
	if (document.forms[0].TQResult.value = "PM")
		{
			document.forms[0].TQScore.value = "2";
		}
	if (document.forms[0].TQResult.value = "M")
		{
			document.forms[0].TQScore.value = "3";
		}
	if (document.forms[0].TQResult.value = "E")
		{
			document.forms[0].TQScore.value = "4";
		}
	if (document.forms[0].TQResult.value = "FE")
		{
			document.forms[0].TQScore.value = "5";
		}

**johnnyi** · 06-21-2005, 02:05 PM

(Insert rolling eyes here)... figured it out... I needed the == instead of = for the statement - should be:

Code:

	if (document.forms[0].TQResult.value == "DNM")
		{
			document.forms[0].TQScore.value = "1";
		}
	if (document.forms[0].TQResult.value == "PM")
		{
			document.forms[0].TQScore.value = "2";
		}
	if (document.forms[0].TQResult.value == "M")
		{
			document.forms[0].TQScore.value = "3";
		}
	if (document.forms[0].TQResult.value == "E")
		{
			document.forms[0].TQScore.value = "4";
		}
	if (document.forms[0].TQResult.value == "FE")
		{
			document.forms[0].TQScore.value = "5";
		}

**mwinter** · 06-21-2005, 02:59 PM

Originally Posted by johnnyi

(Insert rolling eyes here)... figured it out... I needed the == instead of = for the statement

Correct. One way that can help avoid mishaps like this is to place l-values on the right-hand side of a comparison, and r-values on the left.

An l-value is something that can be assigned to, and usually appears to the left of an assignment (=). Examples are variables or object properties. An r-value is something would normally appear to the right of an assignment expression. Examples are function calls and literals, as well as l-values. For this though, we'll ignore l-values.

If you place constants and the like to the left, and you did accidentally create an assignment rather than a comparison, a run-time error would occur, rather than a silent failure in logic.

Code:

if('DNM' = document.forms[0].TQResult.value) {

would fail with an error message because you cannot assign to string literals.

An alternative approach is pass your code through JSLint. This flags syntax errors and potentially error-prone code. If it sees

Code:

if(a = b) {

then it will report an error, no matter what a and b are. If you really did intend to write an assignment, then you can override this behaviour by using another set of parentheses:

Code:

if((a = b)) {

This shows a conscious choice by the author, rather than a typo.

As for the code you posted, there's a slightly better way. When you are comparing the same thing to a range of values, it is better to use a switch statement. This reduces code size and sometimes improves speed, as well. You can also improve efficiency by keeping a reference to form controls, rather than accessing them in full each time:

Code:

var elements = document.forms[0].elements,
    result   = elements.TQResult,
    score    = elements.TQScore;

switch(result.value) {
  case 'DNM': score.value = '1'; break;
  case 'PM':  score.value = '2'; break;
  case 'M':   score.value = '3'; break;
  case 'E':   score.value = '4'; break;
  case 'FE':  score.value = '5'; break;
}

Finally, if a series of conditions are exclusive (for example, value couldn't both be 'M' and 'E'), then your code should reflect that. In other words, rather than several if statements, you should have had a set of if...else if statements:

Code: