Рефакторинг прочь помеченных петель - PullRequest
18 голосов
/ 19 августа 2008

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

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

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

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 );
}     

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

Ответы [ 12 ]

34 голосов
/ 19 августа 2008

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

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

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

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

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

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

1 голос
/ 20 августа 2008

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


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

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

Мои извинения за недоразумение.

1 голос
/ 19 августа 2008

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

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

Я задал SO вопрос о соглашении (с меткой во всех заглавных буквах или нет) для меток в Java.

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

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

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

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

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

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

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

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

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

РЕДАКТИРОВАТЬ: Если подумать, пример на самом деле не ужасное использование меток. Я согласен, что "goto is no-no" , но не из-за такого кода. Использование меток здесь не оказывает существенного влияния на читаемость кода. Конечно, они не являются обязательными и могут быть легко опущены, но не использовать их просто потому, что «использование меток плохо» в данном случае не является хорошим аргументом. В конце концов, удаление меток не делает код намного проще для чтения, как уже прокомментировали другие.

1 голос
/ 19 августа 2008

@ Nicolas

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

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

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

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

1 голос
/ 19 августа 2008

Из чтения вашего кода.

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

Так что вы можете сделать это:

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

Надеюсь, это поможет.

1 голос
/ 19 августа 2008

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

for( int idx = 0; idx 
1 голос
/ 19 августа 2008

Легко, мой хороший человек.

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 );
}

РЕДАКТИРОВАТЬ: Совершенно верно, вы Андерс. Я отредактировал свое решение, чтобы учесть это.

0 голосов
/ 19 августа 2008

@ Sadie

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

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

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

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

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

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

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

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

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

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

0 голосов
/ 19 августа 2008

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

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;
}
Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...