Рефакторинг с удалением помеченных циклов

StackOverflow https://stackoverflow.com/questions/15851

  •  08-06-2019
  •  | 
  •  

Вопрос

После того, как я убедился, что помеченные перерывы / продолжения - это полное "nono". здесь, Мне нужна помощь, чтобы удалить метку из моего кода.

У меня есть квадратная матрица и вектор, которые имеют одинаковую длину.В векторе уже есть некоторые значения, и в зависимости от значений в матрице вектор изменяется в цикле.

Я надеюсь, что фрагмент кода в принципе понятен…

vectorLoop:
for( int idx = 0; idx < vectorLength; idx++) {
    if( conditionAtVectorPosition( v, idx ) ) continue vectorLoop;

    matrixLoop:
    for( rowIdx = 0; rowIdx < n; rowIdx++ ) {
        if( anotherConditionAtVector( v, rowIdx ) ) continue matrixLoop;
        if( conditionAtMatrixRowCol( m, rowIdx, idx ) ) continue vectorLoop;
    }
    setValueInVector( v, idx );
}     

Пожалуйста, убедите меня, что существует более читаемая / лучшая версия без ярлыков.

Это было полезно?

Решение

Рассматривая представленные на данный момент решения:

  • Все они выглядят менее читабельными, чем оригинал, поскольку требуют тратить больше кода на механизм кода, а не на сам алгоритм

  • Некоторые из них сломаны или были сломаны до того, как были отредактированы.Самым ужасным является тот факт, что людям приходится довольно усердно думать о том, как написать код без меток и ничего не сломать.

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

  • Рефакторинг соответствующей части кода в метод фактически является беспроигрышной операцией:это изменяет порядок расположения кода в файле, но никак не влияет на то, как он выполняется.

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

Другие советы

@Patrick вы предполагаете, что вызываете setValueInVector( v, idx);в конце второго цикла все в порядке.Если код должен быть идентичным, логически он должен быть переписан на что-то вроде этого:

for( int idx = 0; idx 

Запросто, мой хороший.

for( int idx = 0; idx < vectorLength; idx++) {
  if( conditionAtVectorPosition( v, idx ) ) continue;

  for( rowIdx = 0; rowIdx < n; rowIdx++ ) {
    if( anotherConditionAtVector( v, rowIdx ) ) continue;
    if( conditionAtMatrixRowCol( m, rowIdx, idx ) ) break;
  }
  if( !conditionAtMatrixRowCol( m, rowIdx, idx ) )
    setValueInVector( v, idx );
}

Редактировать:Совершенно верно, вы Андерс.Я отредактировал свое решение, чтобы учесть и это.

Прочитав ваш код.

  • Я заметил, что вы удалили недопустимые позиции вектора в conditionAtVectorPosition, затем вы удалили недопустимые строки в anotherConditionAtVector.
  • Похоже, что проверка строк в anotherConditionAtVector избыточна, поскольку каким бы ни было значение idx , anotherConditionAtVector зависит только от индекса строки (при условии, что anotherConditionAtVector не имеет побочных эффектов).

Значит, ты можешь это сделать:

  • Сначала получите допустимые позиции, используя conditionAtVectorPosition (это допустимые столбцы).
  • Затем получите допустимые строки, используя anotherConditionAtVector.
  • Наконец, используйте conditionAtMatrixRowCol, используя допустимые столбцы и строки.

Я надеюсь, что это поможет.

@Николас

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

У меня другая точка зрения:некоторые из них не работают, потому что трудно понять поведение исходного алгоритма.

Я понимаю, что это субъективно, но у меня нет никаких проблем с чтением исходного алгоритма.Это короче и понятнее, чем предлагаемые замены.

Что делают все рефакторинги в этом потоке, так это эмулируют поведение метки с использованием функций другого языка - как если бы вы портировали код на язык, в котором нет меток.

Некоторые из них приводят к снижению производительности при повторном запуске одного и того же теста, что не всегда может быть тривиальным.Альтернативой этому является хранение и передача круглых логических значений, что становится уродливым.
Снижение производительности незначительно.Однако я согласен, что повторный запуск теста не является хорошим решением.

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

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

При этом повторное выполнение одного и того же теста дважды не является оптимальным — теоретически.

Редактировать:Если подумать, то этот пример на самом деле не является ужасным использованием ярлыков.Я согласен с тем , что "гото - это ни-ни", но не из-за подобного кода.Использование меток здесь на самом деле не оказывает существенного влияния на читабельность кода.Конечно, они не обязательны и могут быть легко опущены, но отказ от их использования просто потому, что "использование меток - это плохо" не является хорошим аргументом в данном случае.В конце концов, удаление меток не делает код намного легче для чтения, как уже отмечали другие.

Этот вопрос был не об оптимизации алгоритма - но все равно спасибо ;-)

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

Я спросил ТАК а вопрос о соглашении (иметь метку со всеми заглавными буквами или нет) для меток в Java.

В основном каждый ответ говорил мне: "не используй их - всегда есть лучший способ!рефакторинг!".Итак, я опубликовал этот вопрос, чтобы попросить более читаемое (и, следовательно, лучшее?) решение.

До сих пор меня не до конца убедили представленные на данный момент альтернативы.

Пожалуйста, не поймите меня неправильно.Большую часть времени ярлыки - это зло.

Но в моем случае условные тесты довольно просты, а алгоритм взят из математической статьи и поэтому, скорее всего, не изменится в ближайшем будущем.Поэтому я предпочитаю, чтобы все соответствующие части были видны сразу, вместо того чтобы переходить к другому методу с именем что-то вроде checkMatrixAtRow (x).

Особенно в более сложных математических алгоритмах, мне довольно сложно найти "хорошие" имена функций, но я думаю, что это еще один вопрос

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


После того, как я выдвинул обвинение в предложении рефакторинга циклов в исходном вопросе и теперь вижу рассматриваемый код, я думаю, что у вас там есть очень читаемый цикл.

То, что я представлял себе, было совсем другим фрагментом кода - приведя реальный пример, я вижу, что он намного чище, чем я думал.

Приношу свои извинения за возникшее недоразумение.

Работает ли это у вас?Я извлек внутренний цикл в метод CheckedEntireMatrix (вы можете назвать его лучше меня) - Также моя java немного заржавела..но я думаю, что это передает суть дела

for( int idx = 0; idx < vectorLength; idx++) {
    if( conditionAtVectorPosition( v, idx ) 
    || !CheckedEntireMatrix(v)) continue;

    setValueInVector( v, idx );
}

private bool CheckedEntireMatrix(Vector v)
{
    for( rowIdx = 0; rowIdx < n; rowIdx++ ) {
        if( anotherConditionAtVector( v, rowIdx ) ) continue;
        if( conditionAtMatrixRowCol( m, rowIdx, idx ) ) return false;
    }   
    return true;
}

У Гишу есть правильная идея :

for( int idx = 0; idx < vectorLength; idx++) {
    if (!conditionAtVectorPosition( v, idx ) 
        && checkedRow(v, idx))
         setValueInVector( v, idx );
}

private boolean checkedRow(Vector v, int idx) {
    for( rowIdx = 0; rowIdx < n; rowIdx++ ) {
        if( anotherConditionAtVector( v, rowIdx ) ) continue;
        if( conditionAtMatrixRowCol( m, rowIdx, idx ) ) return false;
    }  
    return true;
}

Я не слишком уверен, что понимаю первое продолжение.Я бы скопировал Gishu и написал что-то вроде ( извините, если есть какие- то ошибки) :

for( int idx = 0; idx < vectorLength; idx++) {
    if( !conditionAtVectorPosition( v, idx ) && CheckedEntireMatrix(v))
        setValueInVector( v, idx );
}

inline bool CheckedEntireMatrix(Vector v) {
    for(rowIdx = 0; rowIdx < n; rowIdx++)
        if ( !anotherConditionAtVector(v,rowIdx) && conditionAtMatrixRowCol(m,rowIdx,idx) ) 
            return false;
    return true;
}

@Сэйди:

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

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

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

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

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

Снижение производительности незначительно.Однако я согласен, что повторный запуск теста не является хорошим решением.

Рефакторинг соответствующей части кода в метод фактически является беспроигрышной операцией:это изменяет порядок расположения кода в файле, но никак не влияет на то, как он выполняется.

Я не вижу в этом смысла.Да, это вроде бы не меняет поведение...рефакторинг?

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

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

Лицензировано под: CC-BY-SA с атрибуция
Не связан с StackOverflow
scroll top