Pregunta

Assume i have a function checkTime like the one below where i have to check for multiple condition simultaneously.

var result=0;
function checkTime(time1, time2) {

    if (time1 >= 0 && time2 <= 0) {
        result = 1;
    }
    else if (time1 >= 0 && time2 <= 1) {
        result = 4;
    }
    else if (time1 >= 2 && time2 <= 3) {
        result = 5;
    }
    else if (time1 >= 4 && time2 <= 6) {
        result = 6;
    }
    else if (time1 >= 7 && time2 <= 9) {
        result = 7;
    }
    else if (time1 >= 11 && time2 <= 12) {
        result = 8;
    }
    else if (time1 >= 13 && time2 <= 15) {
        result = 9;
    }
    else if (time1 >= 16 && time2 <= 17) {
        result = 10;
    }
    else if (time1 >= 19 && time2 <= 20) {
        result = 11;
    }
    return result;
}

(The above given example is hypothetical)

The function i have used totally works,but:

  • Is there a better method or procedure or formula to replace this?(where it doesnt have to be this lengthy or ugly)

Thanx!

¿Fue útil?

Solución

You can use an array to represent all the combination:

tests = [
    { time1: 0, time2: 0, result: 1 },
    { time1: 0: time2: 1, result: 4 },
    ...
];

for (var i = 0; i < tests.length; i++) {
    if (time1 >= tests[i].time1 && time2 <= tests[i].time2) {
        return tests[i].result;
    }
}

Otros consejos

If the code is identical, and only the values change, you could do something like this:

function checkTime(time1, time2) {
  [
    [0, 0, 0], 
    [0, 1, 0]
  ].forEach(function (it) {
    if (time1 >= it[0] && time2 <= it[1]) {
       return it[2];
    }
  });
}

Well, first off you have the possibility of an undefined result, so that makes things ambiguous. Should result start at 0 instead? This is an important detail. Second, you seem to be working with boundaries, so it would help to change the <= to < to make this clearer. (If so, the 7-9/11-12 section has a bug.) Third, you have an implicit comparison of time1 and time2, so make that explicit.

var result = 0;
var diff = time2 - time1;
var bounds = [21, 19, 16, 13, 11, 7, 4, 2, 0];
if (diff <= 0) result = 0; // unexpected outcome
else
    for (position = 1; position < bounds.length; ++position) {
        if (time1 >= bounds[position]) {
            if (time2 < bounds[position - 1]) {
                result = 3 + (bounds.size - position);
            }
            break;
        }
    }
return result;

Other implementations are possible, but it's hard to tell based on your question exactly what problem you're solving.

follow-up

This section of code has a gap:

else if (time1 >= 7 && time2 <= 9) {
    result = 7;
}
else if (time1 >= 11 && time2 <= 12) {
    result = 8;
}

If time = 10 and time2 = 10, there is no match. It's easy to miss this type of error when you are repeating yourself. Specifying lower and upper bounds for each condition is unnecessary repetition. Since I couldn't see a pattern to the bounds (which could be delegated to a function), I just put the lower bounds into an array and made sure it was sorted descending so that the loop could stop after the first match.

Licenciado bajo: CC-BY-SA con atribución
No afiliado a StackOverflow
scroll top