2010-12-18 3 views
3

Я иногда нахожусь в этом положении, и я уверен, что есть лучший способ сделать это, чем я сейчас.лучший способ кодирования рекурсивных операторов if

В этом примере я пытаюсь сортировать группу раз, когда у меня конфликтующие элементы. Мне нужно знать, какие времена имеют высокий приоритет, не могут быть перемещены, а также с низким приоритетом, могут быть перемещены. Но я уверен, что этот код очень неэффективен.

var holdLowPriorities = []; 

for (conflict = 0; conflict < sortedTimes.length - 1; conflict++) { 
    var firstConflictTimeStart = sortedTimes[conflict][0]; 
    var firstConflictTimeEnd = sortedTimes[conflict][1]; 
    var secondConflictTimeStart = sortedTimes[conflict + 1][0]; 
    var secondConflictTimeEnd = sortedTimes[conflict + 1][1]; 

    if (firstConflictTimeStart < secondConflictTimeEnd && 
      firstConflictTimeEnd > secondConflictTimeStart) { 
     // are either of the conflicts a high priority 
     var firstContactPriority = sortedTimes[conflict][2]; 
     var secondContactPriority = ortedTimes[conflict + 1][2] 

     if (firstConflictPriority == 2) { 
      //this is high priority, can't move 
     } 
     if (secondConflictPriority == 2) { 
      // this is also a priority, but has to move 
     } 
     // are either of the conflicts a low priority? 
     if (firstConflictPriority == 0) { 
      // this is a low priority so I can adjust the time 
     } else if (secondConflictPriority == 0) { 
      // this is a low priority so I can adjust the time 
     } 
    } 
} 

К сожалению, я даже не знаю, как назвать этот тип проблемы, и поэтому не знаю, что искать, хотя я уверен, что ответ не слишком сложно (я надеюсь, не в любом случае).

+7

Вы можете сделать код более удобным для чтения (и, возможно, более эффективным), используя некоторые промежуточные переменные с точными именами. –

+0

Какова структура 'sortedTimes'? –

+2

Ничего себе, это сложно сохранить блок кода ... 10/10 для того, чтобы его улучшить ... Вы определенно хотите прислушаться к совету Митча Пшеница - и как только вы получите промежуточные переменные, отредактируйте свой Q с обновленным код. – Basic

ответ

2

Заявление switch может помочь немного очистить ваш код.

+0

Тогда ему придется писать несколько switch, bec по какой-то причине он имеет несколько переменных в своих операторах if. Плюс он будет ограничен только строковыми значениями, а не условиями. – Babiker

+0

Это не относится к javascript, операторы switch также принимают выражения. (см. ссылку) –

+0

Что-то новое узнали! +1 – Babiker

1

С точки зрения структур данных, нет ничего изначально неэффективно с if..else-блоками. Даже вложенные if..else не проблема. С точки зрения удобочитаемости это совсем другое дело. Как правило, большие и глубоко вложенные, если ... небезопасные блоки трудно следовать (для человека компьютеры, конечно, не имеют проблем с ними).

Прежде всего, я бы предложил предложение Митча использовать промежуточные переменные, чтобы уменьшить многословность кода. С точки зрения эффективности он определенно менее эффективен с точки зрения памяти (хотя для javascript он может потенциально ускорить доступ в зависимости от браузера). Но эффективность не в этом, ясность кода.

Во-вторых, используйте синтаксис массива массива вместо new Array(): holdLowPriorities.push([...]). Опять же, меньше шума на экране, тем легче видеть, что происходит.

В-третьих, есть несколько мест, где все, что вы делаете, - это проверка приоритета чего-то. Используйте либо вспомогательную функцию, либо метод для упрощения кода здесь: checkProirity(sortedTimes,conflict,2) или sortedTimes.checkProirity(conflict,2). Опять же, вызов функции/метода по своей сути менее эффективен, но дело в том, чтобы улучшить четкость кода для хорошей читаемости.

+0

Спасибо Slebetman, я действительно изменил код во время публикации. Я удалил материал «нового массива», потому что на самом деле это не часть этого вопроса. Использование вспомогательной функции - одно из решений, но я надеюсь, что правильный ответ лучше, чем просто вернуть другую переменную из вспомогательной функции. Но, может быть, это и есть ответ. – pedalpete

2

Редактировать: Вы изменили вопрос, поэтому большая часть этого не имеет значения.

Это простое разъяснение.

var holdLowPriorities = []; 

for(conflict=0; conflict<sortedTimes.length-1; conflict++){ 
    var conflictTime = sortedTimes[conflict], 
     nextConflictTime = sortedTimes[conflict + 1]; 
    if (conflictTime[0] >= nextConflictTime[1] || conflictTime[1] <= nextConflictTime[0]) { 
    continue; 
    } 

    // are either of the conflicts a high priority 
    if (data[conflictTime[2]].stepsArr[conflictTime[3]].priority==2) { 
    alert(data[conflictTime[2]].stepsArr[conflictTime[3]]. 
    } 
    if (data[nextConflictTime[2]].stepsArr[nextConflictTime[3]].priority == 2) { 
    alert(data[nextConflictTime[2]].stepsArr[nextConflictTime[3]]. 
    } 

    // are either of the conflicts a low priority? 
    if (data[conflictTime[2]].stepsArr[conflictTime[3]].priority==0) { 
    holdLowPriorities.push([conflictTime[2], conflictTime[3], conflict]); 
    } else if (data[nextConflictTime[2]].stepsArr[nextConflictTime[3]].priority == 0) { 
    holdLowPriorities.push([nextConflictTime[2], nextConflictTime[3], conflict+1]) 
    } 

    //alert(data[nextConflictTime[2]].stepsArr[nextConflictTime[3]].prerequisite+' '+conflictTime[0]+' '+conflictTime[1]+' '+nextConflictTime[0]+' '+nextConflictTime[1]+' '+data[nextConflictTime[2]].stepsArr[nextConflictTime[3]].taskid+'/'+data[conflictTime[2]].stepsArr[conflictTime[3]].taskid); 
} 

Тогда это можно сделать более очевидным с помощью вспомогательного метода и еще нескольких переменных.

function conflictData(conflict) { 
    return data[conflict[2]].stepsArr[conflict[3]; 
} 

var holdLowPriorities = []; 

for(conflict=0; conflict<sortedTimes.length-1; conflict++){ 
    var conflictTime = sortedTimes[conflict], 
     nextConflictTime = sortedTimes[conflict + 1]; 
    if (conflictTime[0] >= nextConflictTime[1] || conflictTime[1] <= nextConflictTime[0]) { 
    continue; 
    } 

    var thisConflictData = conflictData(conflictTime), 
     nextConflictData = conflictData(nextConflictTime); 

    // are either of the conflicts a high priority 
    if (thisConflictData.priority == 2) { 
    alert(thisConflictData); 
    } 
    if (nextConflictData.priority == 2) { 
    alert(nextConflictData); 
    } 

    // are either of the conflicts a low priority? 
    if (thisConflictData.priority == 0) { 
    holdLowPriorities.push([conflictTime[2], conflictTime[3], conflict]); 
    } else if (nextConflictData.priority == 0) { 
    holdLowPriorities.push([nextConflictTime[2], nextConflictTime[3], conflict+1]) 
    } 

    //alert(nextConflictData.prerequisite + ' ' + conflictTime[0] + ' ' + conflictTime[1] + ' ' + nextConflictTime[0] + ' ' + nextConflictTime[1] + ' ' + nextConflictData.taskid + '/' + thisConflictData.taskid); 
} 

Когда я выражаюсь следующим образом, я думаю, вы можете начать видеть вероятные ошибки; высокий приоритет имеет conflictTime, а nextConflictTime проверяет, что если, в то время как низкий приоритет имеет, если, else if. Это то, что нужно? Возможно, вы сможете поставить этот/следующий конфликтный приоритет в коммутатор или еще раз реорганизовать код. Но я думаю, что сейчас он находится на том этапе, где его можно понять с большей готовностью.

+0

благодарит Криса, мне очень интересно, что большинство рекомендаций касаются читаемости кода, а не другого способа написать код, функцию, о которой я не знал (кроме переключателя) или что-то в этом роде. Может быть, я не так плох программист, как я думал, просто подробный с уродливым кодом. – pedalpete

Смежные вопросы