Какой блок кода «лучше»? - PullRequest
3 голосов
/ 29 июня 2009

Чтобы продвинуть хорошие навыки программирования и повысить эффективность моего кода (читай: «Мы с братом спорим из-за кода»), я предлагаю этот вопрос опытным программистам:

Какой блок кода "лучше"? Для тех, кто не может прочесть код, стоит ли ставить условие внутри цикла for, чтобы уменьшить объем избыточного кода, чем выводить его наружу и создавать 2 цикла for? Обе части кода работают, вопрос заключается в эффективности и удобочитаемости.

    - (NSInteger)eliminateGroup {
            NSMutableArray *blocksToKill = [[NSMutableArray arrayWithCapacity:rowCapacity*rowCapacity] retain];
            NSInteger numOfBlocks = (NSInteger)[self countChargeOfGroup:blocksToKill];
            Block *temp;
            NSInteger chargeTotal = 0;

//Start paying attention here

            if (numOfBlocks > 3) 
                for (NSUInteger i = 0; i < [blocksToKill count]; i++) {
                    temp = (Block *)[blocksToKill objectAtIndex:i];
                    chargeTotal += temp.charge;
                    [temp eliminate];
                    temp.beenCounted = NO;
                }
            }
            else {
                for (NSUInteger i = 0; i < [blocksToKill count]; i++) {
                    temp = (Block *)[blocksToKill objectAtIndex:i];
                    temp.beenCounted = NO;
                }
            }   
            [blocksToKill release];
            return chargeTotal;
        }

Или ...

        - (NSInteger)eliminateGroup {
            NSMutableArray *blocksToKill = [[NSMutableArray arrayWithCapacity:rowCapacity*rowCapacity] retain];
            NSInteger numOfBlocks = (NSInteger)[self countChargeOfGroup:blocksToKill];
            Block *temp;
            NSInteger chargeTotal = 0;

//Start paying attention here

            for (NSUInteger i = 0; i < [blocksToKill count]; i++) {
                temp = (Block *)[blocksToKill objectAtIndex:i];
                if (numOfBlocks > 3) {
                    chargeTotal += temp.charge;
                    [temp eliminate];
                }
                temp.beenCounted = NO;
            }
            [blocksToKill release];
            return chargeTotal;
        }

Имейте в виду, что это для игры. Этот метод вызывается каждый раз, когда пользователь дважды нажимает на экран, и цикл for обычно выполняется в диапазоне от 1 до 15 итераций, максимум 64. Я понимаю, что на самом деле это не имеет большого значения, это главным образом для того, чтобы помочь мне понять, насколько дорогими являются условные утверждения. (Читайте: я просто хочу знать, прав ли я.)

Ответы [ 8 ]

9 голосов
/ 29 июня 2009

Первый блок кода чище и эффективнее, потому что проверка numOfBlocks> 3 является истинной или ложной на протяжении всей итерации.

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

Второй блок можно улучшить, добавив

bool increaseChargeTotal = (numOfBlocks > 3)

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

Лично в этом случае я бы проголосовал за первый вариант (дублированные циклы), потому что тела циклов малы, и это ясно показывает, что условие является внешним по отношению к циклу; Кроме того, он более эффективен и может соответствовать шаблону «сделайте общий случай быстрым».

8 голосов
/ 29 июня 2009

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

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

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

8 голосов
/ 29 июня 2009

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

Однако, еще одна вещь, которую следует учитывать, - это размер кода: подход с двумя циклами генерирует намного больше кода, особенно если большая часть тела цикла велика. Это не только увеличивает размер объектного кода вашей программы, но также снижает производительность кэша инструкций - вы получите гораздо больше промахов в кэше.

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

5 голосов
/ 29 июня 2009

В начале / конце метода вы, вероятно, потратите больше времени на бессмысленные и ненужные [blockToKill retain] / [blocksToKill release], чем на выполнение нескольких десятков сравнений. Нет необходимости сохранять массив, так как он вам не понадобится после возврата, и он никогда не будет очищен до этого.

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

Если добавить рекомендацию Йенса для использования быстрого перечисления и рекомендацию Антти для использования явно логического значения, вы получите что-то вроде:

    - (NSInteger)eliminateGroup {
        NSMutableArray *blocksToKill = [NSMutableArray arrayWithCapacity:rowCapacity*rowCapacity];
        NSInteger numOfBlocks = (NSInteger)[self countChargeOfGroup:blocksToKill];
        NSInteger chargeTotal = 0;

        BOOL calculateAndEliminateBlocks = (numOfBlocks > 3);
        for (Block* block in blocksToKill) {
            if (calculateAndEliminateBlocks) {
                chargeTotal += block.charge;
                [block eliminate];
            }
            block.beenCounted = NO;
        }
        return chargeTotal;
    }

Если вы закончите свой проект и если ваша программа не работает достаточно быстро (два больших if), вы можете профилировать ее и найти «горячие точки», а затем вы можете определить, стоит ли задуматься о тех нескольких микросекундах, которые вы тратите на рассмотрение этой ветви - конечно, сейчас вообще не стоит об этом думать, а это означает, что единственное соображение - это то, что является более читабельным / поддерживаемым.

5 голосов
/ 29 июня 2009

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

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

4 голосов
/ 29 июня 2009

Мой голос решительно за второй блок.

Второй блок проясняет, в чем заключается разница в логике, и имеет ту же самую структуру зацикливания. Он более читабелен и удобен в обслуживании.

Первый блок является примером преждевременной оптимизации .

Что касается использования bool для «сохранения» всех этих сравнений LTE - в этом случае, я не думаю, что это поможет, машинный язык, вероятно, потребует точно такое же количество и размер инструкций.

3 голосов
/ 29 июня 2009

Издержки теста "if" - это несколько инструкций процессора; намного меньше, чем микросекунда. Если вы не думаете, что цикл будет запускаться сотни тысяч раз в ответ на пользовательский ввод, он просто теряется в шуме. Поэтому я бы выбрал второе решение, потому что код меньше и его легче понять.

В любом случае, я бы изменил цикл на

for (temp in blocksToKill) {...}

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

0 голосов
/ 29 июня 2009

Читаемость (и, следовательно, ремонтопригодность) может и должна быть принесена в жертву во имя производительности, , но когда и только когда было определено, что производительность является проблемой .

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

...