38daf62a7887b44454a78a37552b22e4

Logic is cheap and DOM is expensive, so I set an element's current state in a variable and only change the color if its state has changed. The code seems much longer than necessary, though. Passing in true to updateall recolors everything, a rare occurence, otherwise 0 is passed in.

function state_classes(updateall) {
    var value = display.value * 1, tempcolor;
    for (var i = 1; i <= 118; i++) {
        tempcolor = updateall;
        if (!boil[i] && !melt[i]) {
            if (element_ids[i].state !== "Unknown") {
                tempcolor = true;
                element_ids[i].state = "Unknown";
            }
        } else if (value > boil[i]) {
            if (element_ids[i].state !== "Gas") {
                tempcolor = true;
                element_ids[i].state = "Gas";
            }
        } else if (value > melt[i]) {
            if (element_ids[i].state !== "Liquid") {
                tempcolor = true;
                element_ids[i].state = "Liquid";
            }
        } else if (element_ids[i].state !== "Solid") {
            tempcolor = true;
            element_ids[i].state = "Solid";
        }
        if (tempcolor) {
            if (wholetable.inverted)    element_ids[i].style.backgroundColor = statecolors[1][element_ids[i].state];
            else                        element_ids[i].style.color = statecolors[0][element_ids[i].state];
        }
    }
}

Refactorings

No refactoring yet !

5071c5b861341c0dcfcf6ac86327701f

Tien Dung

September 16, 2008, September 16, 2008 00:52, permalink

No rating. Login to rate! 
function state_classes(updateall) {
    var value = display.value * 1, tempcolor;
    var element, state, states = ["Unknown", "Gas", "Liquid", "Solid"];
    for (var i = 1; i <= 118; i++) {
        tempcolor = updateall;
        element = element_ids[i];
        
        for (var j = 0; j < state.length; j++) {
           state = states[j];
           if (element.state !== state) {
             if ( (state === "Unknown" && !boil[i] && !melt[i]) ||
                  (state === "Gas"     && value > boil[i]) ||
                  (state === "Liquid"  && value > melt[i]) ||
                  (state === "Solid")
             {
                tempcolor = true;
                element.state = state;
                break;
             }
           }
        }        

        if (tempcolor) {
          if (wholetable.inverted)    
            element.style.backgroundColor = statecolors[1][element.state];
          else                        
            element.style.color = statecolors[0][element.state];
        }
    }
}
5071c5b861341c0dcfcf6ac86327701f

Tien Dung

September 16, 2008, September 16, 2008 06:44, permalink

No rating. Login to rate!

or using eval(..)

function state_classes(updateall) {
    var value = display.value * 1, tempcolor;
    var element, state, 
        validations = {
          "Unknown" : "!boil[i] && !melt[i]", 
          "Gas"     : "value > boil[i]",
          "Liquid"  : "value > melt[i]", 
          "Solid"   : "true"
        };

    for (var i = 1; i <= 118; i++) {
        tempcolor = updateall;
        element = element_ids[i];
        
        for ( state in validations ) 
         if ( validations.hasOwnProperty(state) && 
              element.state !== state &&
              eval(validations[state]) )
         {
           tempcolor = true;
           element.state = state;
           break;
         }

        if (tempcolor) {
          if (wholetable.inverted)    
            element.style.backgroundColor = statecolors[1][element.state];
          else                        
            element.style.color = statecolors[0][element.state];
        }
    }
}
A8d3f35baafdaea851914b17dae9e1fc

Adam

September 18, 2008, September 18, 2008 04:47, permalink

No rating. Login to rate!

Using Prototype and taking some liberties about your application design.

var State = {
    validations: $H({
        "Unknown": function(index, value) { !boil[index] || !melt[index] },
        "Gas": function(index, value) { value > boil[index] },
        "Liquid": function(index, value) { value > melt[index] },
        "Solid": function(index, value) { true }
    }),
    
    classes: function(updateAll) {
        var value = State.display.value * 1;

        State.elements.each(function(element, index) {
            var refresh = updateAll || State.validations.any(function(state, method) {
                return method(index, value) ? element.state = state : false;
            });
            
            if (refresh) {
                State.refresh(element);
            }
        });
    },
    
    refresh: function(element) {
        if (State.wholeTable.inverted) {
            element.style.backgroundColor = State.colours[1][element.state];
        } else {
            element.style.color = State.colours[0][element.state];
        }
    }
}

Your refactoring





Format Copy from initial code

or Cancel