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

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

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

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

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

Убедите меня, что есть более читабельная / лучшая версия без этикеток.

Пользовательский скаляр GraphQL
Пользовательский скаляр GraphQL
Листовые узлы системы типов GraphQL называются скалярами. Достигнув скалярного типа, невозможно спуститься дальше по иерархии типов. Скалярный тип...
Как вычислять биты и понимать побитовые операторы в Java - объяснение с примерами
Как вычислять биты и понимать побитовые операторы в Java - объяснение с примерами
В компьютерном программировании биты играют важнейшую роль в представлении и манипулировании данными на двоичном уровне. Побитовые операции...
Поднятие тревоги для долго выполняющихся методов в Spring Boot
Поднятие тревоги для долго выполняющихся методов в Spring Boot
Приходилось ли вам сталкиваться с требованиями, в которых вас могли попросить поднять тревогу или выдать ошибку, когда метод Java занимает больше...
Полный курс Java для разработчиков веб-сайтов и приложений
Полный курс Java для разработчиков веб-сайтов и приложений
Получите сертификат Java Web и Application Developer, используя наш курс.
18
0
1 952
12
Перейти к ответу Данный вопрос помечен как решенный

Ответы 12

Легко, мой добрый человек.

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

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

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

for( int idx = 0; idx 

Это работает для вас? Я извлек внутренний цикл в метод 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;
}

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

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

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

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

Итак, вы можете сделать это:

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

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

Ответ принят как подходящий

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

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

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

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

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

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

@ Сэди:

They all look less readable than the original, in that they involve spending more code on the mechanism of the code rather than on the algorithm itself

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

Some of them are broken, or were before they were edited. Most damning is the fact that people are having to think quite hard about how to write the code without labels and not break anything.

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

Some come with a performance penalty of running the same test twice, which may not always be trivial. The alternative to that is storing and passing round booleans, which gets ugly.

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

Refactoring the relevant part of the code into a method is effectively a no-op: it rearranges how the code is laid out in the file, but has no effect on how it's executed.

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

Certainly there are cases where labels are used incorrectly and should be refactored away. I just don't think it should be treated as some unbreakable rule.

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

Это должны быть комментарии. (Я знаю, что функция не выходила, когда это было опубликовано.)

Laurel 28.08.2016 01:01

@ Николас

Some of them are broken, or were before they were edited. Most damning is the fact that people are having to think quite hard about how to write the code without labels and not break anything.

I have a different point of view: some of them are broken because it is hard to figure out the behavior of the original algorithm.

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

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

Some come with a performance penalty of running the same test twice, which may not always be trivial. The alternative to that is storing and passing round booleans, which gets ugly.
The performance penalty is minor. However I agree that running a test twice is not a nice solution.

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

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

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

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

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

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

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

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

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

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

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

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

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


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

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

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

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