Javascript: How to simplify this list of JS styles?

Question

I'm trying to write a simple form. The form basically need to validate 2 mandatory drop down fields upon clicking the submit button by highlighting the labels if nothing was selected.

I have this working fine, but its so long and chunky atm that I must ask, is there a way to simply this?

function submitCheck() {
    if (formTest.connection.value.length==0 && formTest.location.value.length==0) {
        document.getElementById("connection").style.color = "#961515";
        document.getElementById("location").style.color = "#961515";
        document.getElementById("connection").style.fontStyle = "italic";
        document.getElementById("location").style.fontStyle = "italic";
    } else if (formTest.connection.value.length==0 && formTest.location.value.length!=0) {
        document.getElementById("connection").style.color = "#961515";
        document.getElementById("connection").style.fontStyle = "italic";
        document.getElementById("location").style.color = "#000000";
        document.getElementById("location").style.fontStyle = "normal";
    } else if (formTest.location.value.length==0 && !formTest.connection.value.length!=0) {
        document.getElementById("location").style.color = "#961515";
        document.getElementById("location").style.fontStyle = "italic";
        document.getElementById("connection").style.color = "#000000";
        document.getElementById("connection").style.fontStyle = "normal";
    } else {
        document.getElementById("connection").style.color = "#000000";
        document.getElementById("location").style.color = "#000000";
        document.getElementById("location").style.fontStyle = "normal";
        document.getElementById("connection").style.fontStyle = "normal";
        document.getElementById("flashTest").sendValFromHtml(postcodeVal.value);
    }
}

Answer 1

I prefer a table driven approach that reduces the amount of copied and repeatedly executed code:

function submitCheck() {
    // arrays of values
    //   connection color, location color, connection fontStyle, location fontStyle
    var values = [
        ["#961515", "#961515", "italic", "italic"],
        ["#961515", "#000000", "italic", "normal"],
        ["#000000", "#961515", "normal", "italic"],
        ["#000000", "#000000", "normal", "normal"]
    ];

    var conLength = formTest.connection.value.length;
    var locLength = formTest.location.value.length;
    var conn = document.getElementById("connection");
    var loc = document.getElementById("location");
    var index;

    if (conLength == 0 && locLength == 0) {
        index = 0;
    } else if (conLength == 0 && locLength != 0) {
        index = 1;
    } else if (conLength != 0 && locaLength == 0) {
        index = 2;
    } else {
        index = 3;
        document.getElementById("flashTest").sendValFromHtml(postcodeVal.value);
    }

    var data = values[index];
    conn.style.color = data[0];
    loc.style.color = data[1];
    conn.style.fontStyle = data[2];
    loc.style.fontStyle = data[3];
}

Answer 2

You can make a function to set the appropriate styles, rather than repeating everything four times. Also you can store the values you keep testing in variables:

function setStyles(connectionColor, connectionStyle,
                   locationColor, locationStyle) {
    document.getElementById("connection").style.color = connectionColor;
    document.getElementById("location").style.color = locationColor;
    document.getElementById("connection").style.fontStyle = connectionStyle;
    document.getElementById("location").style.fontStyle = locationStyle;
}

function submitCheck() {
    var connectionVal = formTest.connection.value,
        locationVal = formTest.location.value;

    if (connectionVal.length==0 && locationVal.length==0) {
        setStyle("#961515", "italic", "#961515", "italic");
    } else if (connectionVal.length==0 && locationVal.length!=0) {
        setStyle("#961515", "italic", "#000000", "normal");
    } else if (locationVal.length==0 && !connectionVal.length!=0) {
        setStyle("#000000", "normal", "#961515", "italic");
    } else {
        setStyle("#000000", "normal", "#000000", "normal");
        document.getElementById("flashTest").sendValFromHtml(postcodeVal.value);
    }
}

That doesn't in any way change your logic, it just makes it shorter and easier to read. However, you can optimise the logic somewhat too if you test the connection and location fields separately.

Rather than individually setting colours and italic/normal you'd be better off defining some classes in your CSS stylesheet and just adding/removing those classes as appropriate. Not only does it make your JS simpler and easier to read, it avoids having specific colours embedded in your JS.

Answer 3

Assuming the elements are of type input In CSS:

input{
  color: #000000;
  font-style: normal;
}

.invalid {
   color: #961515;
   font-style: italic;
}

In JS:

function submitCheck() {
    var hasConnection = formTest.connection.value.length != 0; 
    var hasLocation = formTest.location.value.length != 0;

    document.getElementById("connection").className = hasConnection ? "" : "invalid";
    document.getElementById("location").className = hasLocation ? "" : "invalid";

    if(hasConnection && hasLocation){
        document.getElementById("flashTest").sendValFromHtml(postcodeVal.value);
    }
}

Note, i'm using a primitive way of setting class names, for a more complete solution use this answer or use a framework like jquery.

I think this is a better approach because it separates the styles from the javascript.

Answer 4

Just make a function out of the 4 lines that get repeated all over:

function setStyles(color1, color2, style1, style2) {
    document.getElementById("connection").style.color = color1;
    document.getElementById("location").style.color = color2;
    document.getElementById("connection").style.fontStyle = style1;
    document.getElementById("location").style.fontStyle = style2;
}

And you can replace each block with a simple function call such as:

setStyles("#961515", "#000000", "italic", "normal");

Your updated code would look something like:

function submitCheck() {
    if (formTest.connection.value.length==0 && formTest.location.value.length==0) {
        setStyles("#961515", "#961515", "italic", "italic");
    } else if (formTest.connection.value.length==0 && formTest.location.value.length!=0) {
        setStyles(...);
    } else {
        ...
    }
}