У меня есть часть кода, которую я хочу оптимизировать для удобства чтения и производительности и охлаждения. Прямо сейчас у меня есть эта уродливая вещь:
if ( cond1 && cond2 && cond3 && !cond4)
{
// do something
}
else if ( cond1 && cond2 && !cond3 && cond4)
{
// do something
}
else if ( cond1 && !cond2 && cond3 && cond4)
{
// do something
}
else if (!cond1 && cond2 && cond3 && cond4)
{
// do something
}
else
{
// do something
}
где cond1, cond2, cond3 и cond4 являются логическими значениями, которые были инициализированы перед блоком кода выше. Я хочу сделать это быстрее, менее уродливым и более крутым.
Я думаю об этом:
int val = (cond1 ? 0 : 1) + 2 * (cond2 ? 0 : 1) + 4 * (cond3 ? 0 : 1) + 8 * (cond4 ? 0 : 1);
if (val == 8)
{
// do something
}
else if (val == 4)
{
// do something
}
else if (val == 2)
{
// do something
}
else if (val == 1)
{
// do something
}
else
{
// do something
}
Это работает или есть недостатки? Есть ли способ лучше? Каков типичный способ достижения желаемого результата при просмотре различных комбинаций нескольких условий?
Ну, самый приятный способ написать, что, вероятно, будет
if(cond1 + cond2 + cond3 + cond4 == 3)
{
if(!cond1)
{
// do something
}
else if(!cond2)
{
// do something
}
else if(!cond3)
{
// do something
}
else // !cond4
{
// do something
}
}
else
{
// do something
}
Я опасаюсь, что эти значения не находятся в массиве.
Так или иначе вы хотите получить свои значения в битовые флаги. То есть вы хотите, чтобы бит был установлен или нет в целочисленном типе для каждого условия. Тогда каждое 4-битное значение в вашем случае представляет одно из ваших вышеуказанных условий ANDed. После этого вы можете использовать оператор switch. Это, возможно, более читаемо, и компилятор часто может оптимизировать его в таблицу переходов. То есть, он просто компенсирует ваш счетчик программ некоторым значением в таблице поиска или чем-то вроде этого, и вам больше не нужно проверять каждую комбинацию значений. Таким образом, ваша проверка на случаи ANDed становится постоянным временем, а не линейным, то есть если вы добавили еще 4 флага, и теперь было 256 комбинаций, а не 16, чем этот метод будет столь же быстрым, как и большой. В качестве альтернативы, если вы не доверяете компилятору, чтобы сделать оператор switch таблицей переходов, вы можете сделать это самостоятельно, используя значение flags в качестве индекса для массива указателей функций. Также, вероятно, стоит отметить, что значения аргумента ORed enum складываются или предварительно вычисляются во время компиляции.
enum {
C1 = 0x1,
C2 = 0x2,
C3 = 0x4,
C4 = 0x8
};
unsigned flags = 0;
flags |= cond1 ? C1 : 0x0;
flags |= cond2 ? C2 : 0x0;
flags |= cond3 ? C3 : 0x0;
flags |= cond4 ? C4 : 0x0;
switch (flags) {
case 0: // !cond1 && !cond2 && !cond3 && !cond4
// do something
break;
case C1: // cond1 && !cond2 && !cond3 && !cond4
// do something
break;
case C2: // !cond1 && cond2 && !cond3 && !cond4
// do something
break;
case C1 | C2: // cond1 && cond2 && !cond3 && !cond4
// do something
break;
case C3: // !cond1 && !cond2 && cond3 && !cond4
// do something
break;
case C1 | C3: // cond1 && !cond2 && cond3 && !cond4
// do something
break;
case C2 | C3: // !cond1 && cond2 && cond3 && !cond4
// do something
break;
case C1 | C2 | C3: // cond1 && cond2 && cond3 && !cond4
// do something
break;
case C4: // !cond1 && !cond2 && !cond3 && cond4
// do something
break;
case C1 | C4: // cond1 && !cond2 && !cond3 && cond4
// do something
break;
case C2 | C4: // !cond1 && cond2 && !cond3 && cond4
// do something
break;
case C1 | C2 | C4: // cond1 && cond2 && !cond3 && cond4
// do something
break;
case C3 | C4: // !cond1 && !cond2 && cond3 && cond4
// do something
break;
case C1 | C3 | C4: // cond1 && !cond2 && cond3 && cond4
// do something
break;
case C2 | C3 | C4: // !cond1 && cond2 && cond3 && cond4
// do something
break;
case C1 | C2 | C3 | C4: // cond1 && cond2 && cond3 && cond4
; // do something
};
Кроме того, это охватывает все комбинации. Если вам просто нужно некоторое подмножество, не стесняйтесь удалять некоторые из этих случаев. Компилятор очень хорош в оптимизации операторов switch. Это, вероятно, быстрее, чем любой умный случайный арифметический трюк, который вы могли бы опрокинуть.
enum {
C1 = 0x1,
C2 = 0x2,
C3 = 0x4,
C4 = 0x8
};
unsigned flags = 0;
flags |= cond1 ? C1 : 0x0;
flags |= cond2 ? C2 : 0x0;
flags |= cond3 ? C3 : 0x0;
flags |= cond4 ? C4 : 0x0;
switch (flags) {
case C1 | C2 | C3: // cond1 && cond2 && cond3 && !cond4
// do something
break;
case C1 | C2 | C4: // cond1 && cond2 && !cond3 && cond4
// do something
break;
case C1 | C3 | C4: // cond1 && !cond2 && cond3 && cond4
// do something
break;
case C2 | C3 | C4: // !cond1 && cond2 && cond3 && cond4
// do something
break;
default:
// do something
;
};
Из моего опыта - наличие ряда длинных операторов if/else указывает на аналогичное, но различимое поведение.
Обычно я пытаюсь ввести интерфейс и классы реализации, которые улавливают это поведение и вызывают метод, который даст желаемый результат.
Это делает код более читабельным - не нужно переходить через сложный поток (который позже может быть вложен). Каждый класс отвечает за реализацию абстрактного метода, а обработанный объект будет иметь статический тип интерфейса, а его динамический тип - наиболее подходящий класс реализации.
положите слева условие, которое имеет наибольшую вероятность быть ложным, делая это, оно продолжается до другого, если не проверять оставшиеся условия, так что вы получаете производительность, в так называемом коротком замыкании
Требование как для читаемого, так и для хорошо работающего кода может быть противоречивым и не имеет простого решения для одного размера. Моя рекомендация состоит в том, чтобы четко определить приоритеты целей, которые вы хотите достичь путем рефакторинга. Что является основным "Почему", почему вы оптимизируете код
использовать declarative table driven aproach для объявления ветвей их условий и других артефактов (что они делают). Используйте генератор кода для создания уродливого кода. Таблицы, как правило, легко читаются (BTW "уродливая вещь", которую вы хотите оптимизировать, легко читается, когда вы читаете ее как таблицу со столбцами [cond1, cond2, cond3, cond4, action])
или не беспокойтесь. В большинстве случаев код ($ → _::.: == > и т.д.) Не может быть прочитан без использования колоризаторов или инструментов для определения текста и других расширителей считывания кода. например Code Rocket может отображать автоматические легко читаемые блок-схемы прямо внутри вашей IDE
точный эффективный метод оптимизации зависит от компилятора и цели. То, что эффективно, может быть найдено наверняка, только путем анализа сгенерированного низкоуровневого машинного кода. например насколько это удобно для аппаратного конвейера. Это может быть различным для архитектур CISC или RICS. В вашем случае комбинации условий особенно важно прогнозирование ветвей. Много раз прямая таблицы перехода может быть более эффективной. Как реальные, так и абстрактные процессоры обычно имеют специальные оптимизированные инструкции для этого случая. С другой стороны, немного другой копируемый/вставленный код может работать лучше, но будет менее поддерживаемым
не знаю. В последнее время я узнал, что определение для классного кода:
Если вы можете использовать С++ 11, вы можете переписать его на основе моей работы для этого более раннего ответа: Как упростить несколько операторов if-else-if в С++:
switch(combine(cond1, cond2, cond3, cond4))
{
case combine(1,1,1,0): do_something(1); break;
case combine(1,1,0,1): do_something(2); break;
case combine(1,0,1,1): do_something(3); break;
case combine(0,1,1,1): do_something(4); break;
default:
do_something_else();
break;
}
Красота заключается в том, что combine полностью оценивается во время компиляции - используя мощности constexpr.
Он также является вариационным (поэтому он поддерживает до количества бит в int_max_t в конфигурации вашего компилятора).
Полный пример: Live On Coliru. Механика (которую вы можете поместить в некоторый заголовок, например logic_combine.hpp):
#include <iostream>
#include <iomanip>
#include <limits>
#include <cstdint>
namespace detail
{
// a little overkill to have a functor here too, but it a good habit™
template <typename T = uintmax_t>
struct to_bitmask_f
{
template <typename... Flags> struct result { typedef T type; };
template <typename... Flags>
typename result<Flags...>::type
constexpr operator()(Flags... flags) const {
static_assert(sizeof...(Flags) < std::numeric_limits<uintmax_t>::digits, "Too many flags for integral representation)");
return impl(flags...);
}
private:
constexpr static inline T impl() { return {}; }
template <typename... Flags>
constexpr static inline T impl(bool b, Flags... more) {
return (b?1:0) + (impl(more...) << (T(1)));
}
};
}
template <typename T = uintmax_t, typename... Flags>
constexpr T combine(Flags... flags)
{
return detail::to_bitmask_f<T>()(flags...);
}
Демонстрация:
void do_something(int i) { std::cout << "something " << i << "\n"; }
void do_something_else() { std::cout << "something else\n"; }
void f(bool cond1, bool cond2, bool cond3, bool cond4) {
switch(combine(cond1, cond2, cond3, cond4))
{
case combine(1,1,1,0): do_something(1); break;
case combine(1,1,0,1): do_something(2); break;
case combine(1,0,1,1): do_something(3); break;
case combine(0,1,1,1): do_something(4); break;
default:
do_something_else();
break;
}
}
int main()
{
// some test-cases
f(1,0,1,0);
f(1,0,0,1);
f(0,1,1,0);
f(0,1,0,1);
f(0,1,1,1);
f(1,1,1,1);
f(1,1,1,0);
f(0,0,0,0);
}
Печать
something else
something else
something else
something else
something 4
something else
something 1
something else
Предлагаемая модификация работает. Вы получаете силу двух точно, когда все условия, кроме одного, являются истинными, а сила двух, которую вы получаете, определяется тем, какое условие является ложным. В общем, вы можете обрабатывать любой короткий список комбинаций true/false таким образом, просто используя двоичные представления комбинаций, которые вы хотите использовать в качестве случаев, которые вы тестируете. Для удобства чтения вы можете написать номера комбинаций в операторах if в двоичном формате вместо десятичного, чтобы вы могли легко определить, какие комбинации истинно/ложных вы проверяете.
Это просто попрошайничество для какой-то таблицы поиска и указателей функций/делегатов. Образец If/elseif/elseif/elseif следует избегать, его трудно читать, его трудно понять.
Замените int val=.... и большую инструкцию для фальшивки с enumb action = getAction(); actions(action)();. Это легко, просто, и если все идет не так, вы можете сразу сосредоточиться на том, что проблема: либо действие закодировано неправильно, либо вы выбрали неправильное действие. Обе возможности легко исследовать, и они будут легко работать.
Конечно, в зависимости от вашего языка и ваших условий вы можете сделать это с помощью интерфейса или подтипирования (полиморфизма), который, естественно, будет выглядеть по-другому, но в основном он сводится к одному и тому же:
Я подозрительно отношусь к четырем "независимым" булерам, которые охватывают 16 случаев, из которых только 4 ожидаются, плюс случай по умолчанию. Может ли какая-либо комбинация действительно быть покрыта? Некоторые из них установлены в одном и том же месте или зависят друг от друга? Если удаляется случай по умолчанию, какой случай обычно ожидается?
Просто тот факт, что вы обычно ищете одно ложное значение из четырех, а не одно истинное значение, указывает на то, что четыре логические строки могут быть не правильной структурой данных.
Если вы используете перечисление с пятью или около того значениями, например notCond1, notCond2, notCond3, notCond4 и notAny? Если вы выполняете какое-то особое поведение, если либо cond1, либо cond2 являются ложными, но не тогда, когда они есть, следует изменить код, который устанавливает каждый из них, чтобы убедиться, что оба значения не являются ложными, и либо вызывают ошибку, либо изменить другой, в соответствии с тем, что приемлемо?
Если у вас действительно нет ограничений на размер кода, я думаю, что стоит добавить некоторую многословность, чтобы убедиться, что состояния данных, которые никогда не должны происходить, никогда не произойдут. Это улучшает как читаемость, так и обработку ошибок.
Выполняется ли код выше в цикле или вложенных циклах, и если да, то имеет ли каждый bool возможность изменять каждый раз при запуске цикла или вы можете переместить некоторые из проверок вне внутреннего цикла?
Может быть, что отдельный код, который вы вызываете для каждой комбинации, действительно имеет что-то общее, что можно учесть. Вы можете ориентироваться на рефакторинг в несколько этапов, что-то вроде этого (для простоты используются только два типа):
if (cond1 && !cond2) {
SomethingGeneral();
Cond1Specific();
}
if (cond2 && !cond1) {
SomethingGeneral();
Cond2Specific();
}
к
if (cond1 != cond2) {
SomethingGeneral();
}
if (cond1) {
Cond1Specific();
}
if (cond2) {
Cond2Specific();
}
to
if (MoreThanOneModeSet()) {
break; // or throw?
}
SomethingGeneral();
if (mode == Mode1) {
Cond1Specific();
}
if (mode == Mode2) {
Cond2Specific();
}
Большинство вышеперечисленных предложений, вероятно, должны привести вас к @amit очень важному и правильному решению. Если вы действительно, действительно не видите ни одного из них действительным, вы, вероятно, должны выбрать @Apriori опрятный и полный способ переключения между всеми возможными комбинациями.
Если длина вашего кода и ваше право разрешить его использование, вы также должны попытаться опубликовать более полный фрагмент кода на codereview.stackexchange.com, предложения, которые вы получите там, будут иметь немного другой фокус и, надеюсь, будут дополнять помощь по SO.
Вы ничего не говорите о сложности ваших условий. Это будет ключевым решающим фактором для меня.
Сейчас я буду игнорировать "быстрее". Я вернусь к этому позже.
Рассмотрим "Шаблон обслуживания" или "Шаблон стратегии" , если:
код:
class CondState {
int x;
int y;
// I'd use bool? or Lazy<bool> in C# but if you don't have
// those constructs use a simple tri-state enum.
// Unknown = 0 is not yet evaluated
// True = 1
// False = 2
TriState cond1Cache = 0;
TriState cond2 = 0;
TriState cond3 = 0;
}
function Rule1(ref CondState state) : bool // returns true when done
{
if (... first complex condition) {
... do complex action
return true;
}
return false;
}
function Rule2(ref CondState state) : bool
function Rule3(ref CondState state) : bool
...
function Rule40(ref CondState state) : bool // returns true when done
// --- in your original function
// build state object
var state = {}
// build the list of rules: a list of function pointers (C)
// or function objects (js/node/python)
var rules = [Rule1, Rule2, Rule3 ..., Rule40]
foreach (r in rules) {
if (rule(state))
break;
}
Это намного более чистый код для "выполнения" правил. Это делает его простым и чистым:
Наконец, позвольте мне вернуться к производительности. Если производительность важна, это имеет смысл только в том случае, если часть условной логики подавляет проблемы производительности более низкого уровня. Конечно, это добавляет OO накладные расходы, и вы должны выделить список (чтобы удерживать функцию refs). Но это шаг к более крупным шаблонам OO: шаблон стратегии или шаблон посетителя, который я рассматриваю как "крутое" решение этой проблемы, когда вы выходите за пределы тривиальных выборок (скажем, более 10 сложных условий).