Как улучшить логику для проверки соответствия 4 логических значений некоторым случаям

118

У меня четыре boolзначения:

bool bValue1;
bool bValue2;
bool bValue3;
bool bValue4;

Допустимые значения:

         Scenario 1 | Scenario 2 | Scenario 3
bValue1: true       | true       | true
bValue2: true       | true       | false
bValue3: true       | true       | false
bValue4: true       | false      | false

Так, например, такой сценарий неприемлем:

bValue1: false
bValue2: true
bValue3: true
bValue4: true

На данный момент я придумал это ifзаявление для обнаружения плохих сценариев:

if(((bValue4 && (!bValue3 || !bValue2 || !bValue1)) ||
   ((bValue3 && (!bValue2 || !bValue1)) ||
   (bValue2 && !bValue1) ||
   (!bValue1 && !bValue2 && !bValue3 && !bValue4))
{
    // There is some error
}

Можно ли улучшить / упростить логику этого оператора?

Эндрю Тракл
источник
8
Я бы использовал таблицу вместо сложного ifоператора. Кроме того, поскольку это логические флаги, вы можете моделировать каждый сценарий как константу и проверять его.
Здеслав Войкович 03
3
if (!((bValue1 && bValue2 && bValue3) || (bValue1 && !bValue2 && !bValue3 && !bValue4)))
mch
14
каковы сценарии на самом деле? Часто все становится намного проще, если вы просто даете вещам собственные имена, напримерbool scenario1 = bValue1 && bValue2 && bValue3 && bValue4;
idclev 463035818 03
6
Используя осмысленные имена, вы можете выделить каждое сложное условие в метод и вызвать этот метод в условии if. Это было бы намного удобнее для чтения и обслуживания. например. Взгляните на пример, представленный по ссылке. refactoring.guru/decompose-conditional
Hardik Modha
5
К вашему сведению en.wikipedia.org/wiki/Karnaugh_map
00__00__00 05

Ответы:

195

Я бы стремился к удобочитаемости: у вас всего 3 сценария, разберитесь с ними с 3 отдельными if:

bool valid = false;
if (bValue1 && bValue2 && bValue3 && bValue4)
    valid = true; //scenario 1
else if (bValue1 && bValue2 && bValue3 && !bValue4)
    valid = true; //scenario 2
else if (bValue1 && !bValue2 && !bValue3 && !bValue4)
    valid = true; //scenario 3

Легко читать и отлаживать, ИМХО. Кроме того, вы можете присвоить переменную whichScenario, продолжая работу с if.

Имея всего 3 сценария, я бы не пошел с чем-то вроде «если первые 3 значения верны, я могу избежать проверки четвертого значения»: это затруднит чтение и сопровождение вашего кода.

Не изящное решение может быть конечно, но в этом случае нормально: легко и читабельно.

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

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

(Почти) не по теме:

Я не пишу много ответов здесь, в StackOverflow. Это действительно забавно, что принятый выше ответ, безусловно, является самым ценным ответом в моей истории (я никогда не получал больше 5-10 голосов), хотя на самом деле это не то, что я обычно считаю «правильным» способом сделать это.

Но простота часто является «правильным способом сделать это», многие люди, кажется, думают так, и я должен думать об этом больше, чем я :)

Джан Паоло
источник
1
конечно @hessamhedieh, это нормально только для небольшого количества доступных сценариев. как я уже сказал, если что-то станет более сложным, лучше поищите что-нибудь еще
Джан Паоло
4
Это можно еще больше упростить, поместив все условия в инициализатор validи разделив их с помощью ||вместо изменения validв отдельных блоках операторов. Я не могу привести пример в комментарии, но вы можете вертикально выровнять ||операторы по левому краю, чтобы сделать это очень понятным; отдельные условия уже заключены в круглые скобки столько, сколько они должны быть (для if), поэтому вам не нужно добавлять какие-либо символы в выражения, кроме того, что уже есть.
Леушенко 03
1
@Leushenko, я думаю, что круглые скобки, && и || Условия довольно подвержены ошибкам (кто-то в другом ответе сказал, что в коде OP есть ошибка в скобках, возможно, она была исправлена). Конечно, правильное выравнивание может помочь. Но в чем преимущество? более читабельный? легче поддерживать? Я так не думаю. Просто мое мнение, конечно. Будьте уверены, я действительно ненавижу много "если" в коде.
Джан Паоло
3
Я бы обернул это в форму, if($bValue1)поскольку это всегда должно быть правдой, технически допускающее небольшое улучшение производительности (хотя здесь мы говорим о незначительных суммах).
Martijn
2
FWIW: есть только 2 сценария: первые 2 являются одним и тем же сценарием и не зависят отbValue4
Dancrumb
123

Я бы стремился к простоте и удобочитаемости.

bool scenario1 = bValue1 && bValue2 && bValue3 && bValue4;
bool scenario2 = bValue1 && bValue2 && bValue3 && !bValue4;
bool scenario3 = bValue1 && !bValue2 && !bValue3 && !bValue4;

if (scenario1 || scenario2 || scenario3) {
    // Do whatever.
}

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

bool scenario1or2 = bValue1 && bValue2 && bValue3;
bool scenario3 = bValue1 && !bValue2 && !bValue3 && !bValue4;

if (scenario1or2 || scenario3) {
    // Do whatever.
}

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

Андерс
источник
3
+1 Я бы тоже так поступил. Как указывает @RedFilter, и в отличие от принятого ответа, это самодокументируется. Присвоение сценариям собственных имен на отдельном шаге гораздо удобнее для чтения.
Андреас
106

Мы можем использовать карту Карно и свести ваши сценарии к логическому уравнению. Я использовал решатель карт Карно Online со схемой для 4 переменных.

введите описание изображения здесь

Это дает:

введите описание изображения здесь

Переходя A, B, C, Dна bValue1, bValue2, bValue3, bValue4, это не что иное, как:

bValue1 && bValue2 && bValue3 || bValue1 && !bValue2 && !bValue3 && !bValue4

Итак, ваше ifутверждение становится:

if(!(bValue1 && bValue2 && bValue3 || bValue1 && !bValue2 && !bValue3 && !bValue4))
{
    // There is some error
}
  • Карты Карно особенно полезны, когда у вас есть много переменных и много условий, которые необходимо оценить true.
  • После сведения trueсценариев к логическому уравнению добавление соответствующих комментариев, указывающих на trueсценарии, является хорошей практикой.
PW
источник
96
Хотя этот код технически верен, он требует множества комментариев, чтобы его отредактировал другой разработчик через несколько месяцев.
Здеслав Войкович
22
@ZdeslavVojkovic: Я бы просто добавил комментарий к уравнению. //!(ABC + AB'C'D') (By K-Map logic), Это было бы хорошее время для разработчика изучить K-Maps, если он еще не знает их.
PW
11
Я согласен с этим, но проблема IMO в том, что он не четко сопоставляется с проблемной областью, т.е. как каждое условие сопоставляется с конкретным сценарием, что затрудняет изменение / расширение. Что происходит , когда есть Eи Fусловия , и 4 новых сценариев? Сколько времени нужно, чтобы ifправильно обновить эту инструкцию? Как проверка кода проверяет, все ли в порядке? Проблема не в технической стороне, а в «деловой» стороне.
Здеслав Войкович 03
7
Я думаю, вы можете исключить A: ABC + AB'C'D' = A(BC + B'C'D')(это можно даже учесть, A(B ^ C)'(C + D')хотя я бы осторожно назвал это «упрощением»).
Maciej Piechotka
28
@PW Этот комментарий кажется таким же понятным, как и код, и поэтому немного бессмысленен. Лучший комментарий объяснил бы, как вы на самом деле пришли к этому уравнению, т.е. что оператор должен запускаться для TTTT, TTTF и TFFF. В этот момент вы могли бы просто написать эти три условия в коде и вообще не нуждаться в объяснении.
Бернхард Баркер
58

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

Я бы предложил моделировать это как битовые флаги:

const int SCENARIO_1 = 0x0F; // 0b1111 if using c++14
const int SCENARIO_2 = 0x0E; // 0b1110
const int SCENARIO_3 = 0x08; // 0b1000

bool bValue1 = true;
bool bValue2 = false;
bool bValue3 = false;
bool bValue4 = false;

// boolean -> int conversion is covered by standard and produces 0/1
int scenario = bValue1 << 3 | bValue2 << 2 | bValue3 << 1 | bValue4;
bool match = scenario == SCENARIO_1 || scenario == SCENARIO_2 || scenario == SCENARIO_3;
std::cout << (match ? "ok" : "error");

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

int scenarios[3][4] = {
    {true, true, true, true},
    {true, true, true, false},
    {true, false, false, false},
};

int main()
{
  bool bValue1 = true;
  bool bValue2 = false;
  bool bValue3 = true;
  bool bValue4 = true;
  bool match = false;

  // depending on compiler, prefer std::size()/_countof instead of magic value of 4
  for (int i = 0; i < 4 && !match; ++i) {
    auto current = scenarios[i];
    match = bValue1 == current[0] && 
            bValue2 == current[1] && 
            bValue3 == current[2] && 
            bValue4 == current[3];
  }

  std::cout << (match ? "ok" : "error");
}
Здеслав Войкович
источник
4
Не самый удобный в обслуживании, но определенно упрощает условие if. Так что оставить несколько комментариев вокруг побитовых операций здесь, imo, будет абсолютной необходимостью.
Адам Захран
6
ИМО, таблица - лучший подход, поскольку она лучше масштабируется с дополнительными сценариями и флагами.
Здеслав Войкович 03
Мне нравится ваше первое решение, легко читаемое и открытое для модификации. Я бы сделал 2 улучшения: 1: присвоить значения сценарию X с явным указанием используемых логических значений, например SCENARIO_2 = true << 3 | true << 2 | true << 1 | false;2: избегать переменных SCENARIO_X, а затем сохранить все доступные сценарии в файле <std::set<int>. Добавление сценария будет чем-то вроде mySet.insert( true << 3 | false << 2 | true << 1 | false;небольшого перегиба для всего 3 сценария, OP принял быстрое, грязное и простое решение, которое я предложил в своем ответе.
Джан Паоло
4
Если вы используете C ++ 14 или выше, я бы предложил вместо этого использовать двоичные литералы для первого решения - 0b1111, 0b1110 и 0b1000 намного понятнее. Возможно, вы также можете немного упростить это, используя стандартную библиотеку ( std::find?).
Бернхард Баркер
2
Я считаю, что здесь двоичные литералы являются минимальным требованием для очистки первого кода. В нынешнем виде это совершенно загадочно. Описательные идентификаторы могут помочь, но я даже в этом не уверен. Фактически, битовые операции для получения scenarioзначения кажутся мне излишне подверженными ошибкам.
Конрад Рудольф
27

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

Начиная с ответа @ZdeslavVojkovic (который я считаю неплохим), я придумал следующее:

#include <iostream>
#include <set>

//using namespace std;

int GetScenarioInt(bool bValue1, bool bValue2, bool bValue3, bool bValue4)
{
    return bValue1 << 3 | bValue2 << 2 | bValue3 << 1 | bValue4;
}
bool IsValidScenario(bool bValue1, bool bValue2, bool bValue3, bool bValue4)
{
    std::set<int> validScenarios;
    validScenarios.insert(GetScenarioInt(true, true, true, true));
    validScenarios.insert(GetScenarioInt(true, true, true, false));
    validScenarios.insert(GetScenarioInt(true, false, false, false));

    int currentScenario = GetScenarioInt(bValue1, bValue2, bValue3, bValue4);

    return validScenarios.find(currentScenario) != validScenarios.end();
}

int main()
{
    std::cout << IsValidScenario(true, true, true, false) << "\n"; // expected = true;
    std::cout << IsValidScenario(true, true, false, false) << "\n"; // expected = false;

    return 0;
}

Смотрите его на работе здесь

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

Джан Паоло
источник
Вы знаете, что всегда можете изменить свой предыдущий ответ и внести улучшения.
Андреас
20

Я также хотел бы предложить другой подход.

Моя идея состоит в том, чтобы преобразовать bools в целое число, а затем сравнить, используя вариативные шаблоны:

unsigned bitmap_from_bools(bool b) {
    return b;
}
template<typename... args>
unsigned bitmap_from_bools(bool b, args... pack) {
    return (bitmap_from_bools(b) << sizeof...(pack)) | bitmap_from_bools(pack...);
}

int main() {
    bool bValue1;
    bool bValue2;
    bool bValue3;
    bool bValue4;

    unsigned summary = bitmap_from_bools(bValue1, bValue2, bValue3, bValue4);

    if (summary != 0b1111u && summary != 0b1110u && summary != 0b1000u) {
        //bad scenario
    }
}

Обратите внимание, как эта система может поддерживать до 32 логических значений в качестве входных данных. заменяя unsignedс unsigned long long(или uint64_t) увеличивает поддержку 64 случаев. Если вам не нравится if (summary != 0b1111u && summary != 0b1110u && summary != 0b1000u), вы также можете использовать еще один метод вариативного шаблона:

bool equals_any(unsigned target, unsigned compare) {
    return target == compare;
}
template<typename... args>
bool equals_any(unsigned target, unsigned compare, args... compare_pack) {
    return equals_any(target, compare) ? true : equals_any(target, compare_pack...);
}

int main() {
    bool bValue1;
    bool bValue2;
    bool bValue3;
    bool bValue4;

    unsigned summary = bitmap_from_bools(bValue1, bValue2, bValue3, bValue4);

    if (!equals_any(summary, 0b1111u, 0b1110u, 0b1000u)) {
        //bad scenario
    }
}
Стек Дэнни
источник
2
Мне нравится этот подход, за исключением названия основной функции: «from bool… to what ?» - Почему не явно bitmap_from_bools, или bools_to_bitmap?
Конрад Рудольф
да @KonradRudolph, я не мог придумать лучшего имени, кроме, может быть bools_to_unsigned. Растровое изображение - хорошее ключевое слово; изм.
Stack Danny
Я думаю ты хочешь summary!= 0b1111u &&.... a != b || a != cвсегда верно, еслиb != c
MooseBoys 05
17

Вот упрощенная версия:

if (bValue1 && (bValue2 == bValue3) && (bValue2 || !bValue4)) {
    // acceptable
} else {
    // not acceptable
}

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


Обновление: MSalters в комментариях нашел еще более простое выражение:

if (bValue1&&(bValue2==bValue3)&&(bValue2>=bValue4)) ...
Геза
источник
1
Да, но трудно понять. Но спасибо за предложение.
Эндрю Тракл
Я сравнил способность компиляторов упрощать выражения с вашим упрощением в качестве справочника: обозреватель компилятора . gcc не нашел оптимальной версии, но ее решение все еще хорошее. Clang и MSVC, похоже, не выполняют никакого упрощения логических выражений.
Oliv
1
@AndrewTruckle: обратите внимание, что если вам нужна более читаемая версия, скажите об этом. Вы сказали «упрощенный», но соглашаетесь с еще более подробной версией, чем исходная.
geza 03
1
simpleдействительно расплывчатый термин. Многие люди понимают это в этом контексте как более простое для понимания разработчиком, а не компилятором для генерации кода, поэтому более подробное может быть проще.
Здеслав Войкович 03
1
@IsmaelMiguel: когда логическая формула оптимизируется по количеству терминов, исходное значение обычно теряется. Но вокруг него можно поставить комментарий, чтобы было понятно, что он делает. Даже для принятого ответа комментарий не повредит.
geza 04
12

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

template<class T0>
auto is_any_of( T0 const& t0, std::initializer_list<T0> il ) {
  for (auto&& x:il)
    if (x==t0) return true;
  return false;
}

сейчас

if (is_any_of(
  std::make_tuple(bValue1, bValue2, bValue3, bValue4),
  {
    {true, true, true, true},
    {true, true, true, false},
    {true, false, false, false}
  }
))

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

Живой пример .

Вы также можете использовать std::any_ofнапрямую:

using entry = std::array<bool, 4>;
constexpr entry acceptable[] = 
  {
    {true, true, true, true},
    {true, true, true, false},
    {true, false, false, false}
  };
if (std::any_of( begin(acceptable), end(acceptable), [&](auto&&x){
  return entry{bValue1, bValue2, bValue3, bValue4} == x;
}) {
}

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

Якк - Адам Неврамонт
источник
Первую версию так легко читать и поддерживать, что она мне очень нравится. Второй труднее читать, по крайней мере, для меня, и для него требуется уровень навыков C ++, возможно, выше среднего, наверняка выше моего. Не все могут написать. Только что узнал что-то новое, спасибо
Джан Паоло
11

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

В конце концов, я решил добавить три новых "сценарных" booleanметода:

bool CChristianLifeMinistryValidationDlg::IsFirstWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
    return (INCLUDE_ITEM1(pEntry) && 
           !INCLUDE_ITEM2(pEntry) && 
           !INCLUDE_ITEM3(pEntry) && 
           !INCLUDE_ITEM4(pEntry));
}

bool CChristianLifeMinistryValidationDlg::IsSecondWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
    return (INCLUDE_ITEM1(pEntry) &&
            INCLUDE_ITEM2(pEntry) &&
            INCLUDE_ITEM3(pEntry) &&
            INCLUDE_ITEM4(pEntry));
}

bool CChristianLifeMinistryValidationDlg::IsOtherWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
    return (INCLUDE_ITEM1(pEntry) && 
            INCLUDE_ITEM2(pEntry) && 
            INCLUDE_ITEM3(pEntry) && 
           !INCLUDE_ITEM4(pEntry));
}

Затем я смог применить эти мои процедуры проверки следующим образом:

if (!IsFirstWeekStudentItems(pEntry) && !IsSecondWeekStudentItems(pEntry) && !IsOtherWeekStudentItems(pEntry))
{
    ; Error
}

В моем живом приложении 4 значения bool фактически извлекаются из a, в DWORDкотором закодированы 4 значения.

Еще раз спасибо всем.

Эндрю Тракл
источник
1
Спасибо, что поделились решением. :) Это на самом деле лучше, чем сложные адские условия. Может быть, вы еще можете назвать и INCLUDE_ITEM1т. Д. Лучше, и у вас все в порядке. :)
Hardik Modha 03
1
@HardikModha Ну, технически это «предметы для учащихся», и флаг указывает, должны ли они быть «включены». Так что я думаю, что название, хотя и звучит обобщенно, на самом деле имеет значение в этом контексте. :)
Эндрю Тракл 03
11

Я не вижу никаких ответов, в которых говорится, что нужно назвать сценарии, хотя решение OP делает именно это.

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

Если вы планируете повторно использовать эти сценарии вне своей функции (или можете захотеть это сделать), создайте функцию, которая сообщает, что она оценивает ( constexpr/ noexceptнеобязательно, но рекомендуется):

constexpr bool IsScenario1(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && b2 && b3 && b4; }

constexpr bool IsScenario2(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && b2 && b3 && !b4; }

constexpr bool IsScenario3(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && !b2 && !b3 && !b4; }

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

const auto is_scenario_1 = bValue1 && bValue2 && bValue3 && bValue4;
const auto is_scenario_2 = bvalue1 && bvalue2 && bValue3 && !bValue4;
const auto is_scenario_3 = bValue1 && !bValue2 && !bValue3 && !bValue4;

Компилятор, скорее всего, решит, что если bValue1 ложно, то все сценарии ложны. Не беспокойтесь о том, чтобы сделать его быстрым, просто правильным и читаемым. Если вы профилируете свой код и обнаруживаете, что это узкое место, потому что компилятор сгенерировал неоптимальный код с -O2 или выше, попробуйте его переписать.

ошибочный
источник
Мне это нравится немного больше, чем решение Джана Паоло (уже хорошее): оно позволяет избежать потока управления и использования перезаписываемой переменной - более функциональный стиль.
Dirk Herrmann
9

AC / C ++ способ

bool scenario[3][4] = {{true, true, true, true}, 
                        {true, true, true, false}, 
                        {true, false, false, false}};

bool CheckScenario(bool bValue1, bool bValue2, bool bValue3, bool bValue4)
{
    bool temp[] = {bValue1, bValue2, bValue3, bValue4};
    for(int i = 0 ; i < sizeof(scenario) / sizeof(scenario[0]); i++)
    {
        if(memcmp(temp, scenario[i], sizeof(temp)) == 0)
            return true;
    }
    return false;
}

Этот подход является масштабируемым, так как если количество допустимых условий растет, вы легко просто добавляете их в список сценариев.

Hessam Hedieh
источник
Хотя я почти уверен, что это неправильно. Предполагается, что компилятор использует только одно двоичное представление для true. Компилятор, который использует «все, что ненулевое, истинно», вызывает сбой этого кода. Обратите внимание, что trueнеобходимо преобразовать в 1, его просто не нужно хранить как таковой.
MSalters 03
@MSalters, tnx, я понимаю вашу точку зрения, и я знаю, что, вроде как 2 is not equal to true but evaluates to true, мой код не int 1 = trueработает и работает, пока все true преобразуются в одно и то же значение int, поэтому вот мой вопрос: почему компилятор должен действовать случайным образом при преобразовании верен базовому типу int. Не могли бы вы подробнее рассказать?
hessam hedieh 03
Выполнение a memcmpдля проверки логических условий - это не способ C ++, и я также сомневаюсь, что это установленный способ C.
Конрад Рудольф
@hessamhedieh: Проблема в вашей логике "преобразование true в базовый int". Это не как работают компиляторы,
MSalters
Ваш код увеличивает сложность с O (1) до O (n). Ни на каких языках не выходить - оставьте в стороне C / C ++.
mabel
9

Легко заметить, что первые два сценария похожи - они разделяют большинство условий. Если вы хотите выбрать, в каком сценарии вы находитесь в данный момент, вы можете написать это так (это модифицированное решение @ gian-paolo ):

bool valid = false;
if(bValue1 && bValue2 && bValue3)
{
    if (bValue4)
        valid = true; //scenario 1
    else if (!bValue4)
        valid = true; //scenario 2
}
else if (bValue1 && !bValue2 && !bValue3 && !bValue4)
    valid = true; //scenario 3

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

bool valid = false;
if(bValue1)
{
    if(bValue2 && bValue3)
    {
        if (bValue4)
            valid = true; //scenario 1
        else if (!bValue4)
            valid = true; //scenario 2
    }
    else if (!bValue2 && !bValue3 && !bValue4)
        valid = true; //scenario 3
}

Более того, теперь вы можете ясно видеть, что bValue2 и bValue3 в некоторой степени связаны - вы можете извлечь их состояние в некоторые внешние функции или переменные с более подходящим именем (хотя это не всегда легко или уместно):

bool valid = false;
if(bValue1)
{
    bool bValue1and2 = bValue1 && bValue2;
    bool notBValue1and2 = !bValue2 && !bValue3;
    if(bValue1and2)
    {
        if (bValue4)
            valid = true; //scenario 1
        else if (!bValue4)
            valid = true; //scenario 2
    }
    else if (notBValue1and2 && !bValue4)
        valid = true; //scenario 3
}

У этого способа есть некоторые преимущества и недостатки:

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

Если вы прогнозируете, что в приведенную выше логику произойдут изменения, вам следует использовать более простой подход, представленный @ gian-paolo .

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

Михал Лос
источник
7

Как предлагает mch, вы можете:

if(!((bValue1 && bValue2 && bValue3) || 
  (bValue1 && !bValue2 && !bValue3 && !bValue4))
)

где первая строка охватывает два первых хороших случая, а вторая строка - последний.

Live Demo, где я поигрался и он твои дела проходит.

gsamaras
источник
7

Небольшая вариация прекрасного ответа @GianPaolo, который некоторым может быть легче прочитать:

bool any_of_three_scenarios(bool v1, bool v2, bool v3, bool v4)
{
  return (v1 &&  v2 &&  v3 &&  v4)  // scenario 1
      || (v1 &&  v2 &&  v3 && !v4)  // scenario 2
      || (v1 && !v2 && !v3 && !v4); // scenario 3
}

if (any_of_three_scenarios(bValue1,bValue2,bValue3,bValue4))
{
  // ...
}
Matt
источник
7

Каждый ответ слишком сложен и труден для чтения. Лучшее решение этого - switch()заявление. Он удобен для чтения и упрощает добавление / изменение дополнительных случаев. Компиляторы тоже хороши в оптимизации switch()операторов.

switch( (bValue4 << 3) | (bValue3 << 2) | (bValue2 << 1) | (bValue1) )
{
    case 0b1111:
        // scenario 1
        break;

    case 0b0111:
        // scenario 2
        break;

    case 0b0001:
        // scenario 3
        break;

    default:
        // fault condition
        break;
}

Вы, конечно, можете использовать константы и OR их вместе в caseоператорах для еще большей читабельности.

shogged
источник
Будучи старым программистом на C, я бы определил макрос «PackBools» и использовал бы его как для «switch (PackBools (a, b, c, d))», так и для случаев, например, либо напрямую »case PackBools (true , true ...) "или определите их как локальные константы. например," const unsigned int script1 = PackBools (true, true ...); "
Simon F
6

Я бы также использовал быстрые переменные для ясности. Как отмечалось ранее, сценарий 1 равен сценарию 2, поскольку значение bValue4 не влияет на истинность этих двух сценариев.

bool MAJORLY_TRUE=bValue1 && bValue2 && bValue3
bool MAJORLY_FALSE=!(bValue2 || bValue3 || bValue4)

тогда ваше выражение становится:

if (MAJORLY_TRUE || (bValue1 && MAJORLY_FALSE))
{
     // do something
}
else
{
    // There is some error
}

Присвоение значимых имен переменным MAJORTRUE и MAJORFALSE (а также фактически переменным bValue *) очень поможет с удобочитаемостью и обслуживанием.

Gnudiff
источник
6

Сосредоточьтесь на удобочитаемости проблемы, а не на конкретном «если».

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

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

#include <iostream>
#include <vector>
using namespace std;

// These values would likely not come from a single struct in real life
// Instead, they may be references to other booleans in other systems
struct Values
{
    bool bValue1; // These would be given better names in reality
    bool bValue2; // e.g. bDidTheCarCatchFire
    bool bValue3; // and bDidTheWindshieldFallOff
    bool bValue4;
};

class Scenario
{
public:
    Scenario(Values& values)
    : mValues(values) {}

    virtual operator bool() = 0;

protected:
    Values& mValues;    
};

// Names as examples of things that describe your "scenarios" more effectively
class Scenario1_TheCarWasNotDamagedAtAll : public Scenario
{
public:
    Scenario1_TheCarWasNotDamagedAtAll(Values& values) : Scenario(values) {}

    virtual operator bool()
    {
        return mValues.bValue1
        && mValues.bValue2
        && mValues.bValue3
        && mValues.bValue4;
    }
};

class Scenario2_TheCarBreaksDownButDidntGoOnFire : public Scenario
{
public:
    Scenario2_TheCarBreaksDownButDidntGoOnFire(Values& values) : Scenario(values) {}

    virtual operator bool()
    {
        return mValues.bValue1
        && mValues.bValue2
        && mValues.bValue3
        && !mValues.bValue4;
    }   
};

class Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere : public Scenario
{
public:
    Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere(Values& values) : Scenario(values) {}

    virtual operator bool()
    {
        return mValues.bValue1
        && !mValues.bValue2
        && !mValues.bValue3
        && !mValues.bValue4;
    }   
};

Scenario* findMatchingScenario(std::vector<Scenario*>& scenarios)
{
    for(std::vector<Scenario*>::iterator it = scenarios.begin(); it != scenarios.end(); it++)
    {
        if (**it)
        {
            return *it;
        }
    }
    return NULL;
}

int main() {
    Values values = {true, true, true, true};
    std::vector<Scenario*> scenarios = {
        new Scenario1_TheCarWasNotDamagedAtAll(values),
        new Scenario2_TheCarBreaksDownButDidntGoOnFire(values),
        new Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere(values)
    };

    Scenario* matchingScenario = findMatchingScenario(scenarios);

    if(matchingScenario)
    {
        std::cout << matchingScenario << " was a match" << std::endl;
    }
    else
    {
        std::cout << "No match" << std::endl;
    }

    // your code goes here
    return 0;
}

источник
5
В какой-то момент многословие начинает ухудшать читабельность. Я думаю, это заходит слишком далеко.
JollyJoker 03
2
@JollyJoker Я действительно согласен с этой конкретной ситуацией - однако, мое внутреннее ощущение от того, как OP назвал все в очень общем виде, состоит в том, что их «настоящий» код, вероятно, намного сложнее, чем приведенный ими пример. На самом деле, я просто хотел представить эту альтернативу, так как я бы структурировал ее для чего-то гораздо более сложного / запутанного. Но вы правы - для конкретного примера OP это слишком многословно и усугубляет ситуацию.
5

Это зависит от того, что они представляют.

Например, если 1 - это ключ, а 2 и 3 - два человека, которые должны согласиться (кроме тех случаев, когда они согласны, NOTим нужен третий человек - 4 - для подтверждения), наиболее читаемым может быть:

1 &&
    (
        (2 && 3)   
        || 
        ((!2 && !3) && !4)
    )

по многочисленным просьбам:

Key &&
    (
        (Alice && Bob)   
        || 
        ((!Alice && !Bob) && !Charlie)
    )
ispiro
источник
2
Возможно, вы правы, но использование цифр для иллюстрации вашей точки зрения отвлекает от вашего ответа. Попробуйте использовать описательные имена.
jxh 03
1
@jxh Это числа, используемые OP. Я только что удалил bValue.
ispiro 03
@jxh Надеюсь, теперь стало лучше.
ispiro 05
4

Побитовая операция выглядит очень чисто и понятно.

int bitwise = (bValue4 << 3) | (bValue3 << 2) | (bValue2 << 1) | (bValue1);
if (bitwise == 0b1111 || bitwise == 0b0111 || bitwise == 0b0001)
{
    //satisfying condition
}
Дервиш Кайымбашиоглу
источник
1
Поразрядное сравнение мне кажется читаемым. Композиция же выглядит искусственно.
xtofl 05
3

Я обозначаю a, b, c, d для ясности и A, B, C, D для дополнений.

bValue1 = a (!A)
bValue2 = b (!B)
bValue3 = c (!C)
bValue4 = d (!D)

Уравнение

1 = abcd + abcD + aBCD
  = a (bcd + bcD + BCD)
  = a (bc + BCD)
  = a (bcd + D (b ^C))

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

Мейбл
источник
3
If (!bValue1 || (bValue2 != bValue3) || (!bValue4 && bValue2))
{
// you have a problem
}
  • b1 всегда должен быть правдой
  • b2 всегда должно быть равно b3
  • и b4 не может быть ложным, если b2 (и b3) истинны

просто

Оуэн Мейер
источник
3

Просто личное предпочтение перед принятым ответом, но я бы написал:

bool valid = false;
// scenario 1
valid = valid || (bValue1 && bValue2 && bValue3 && bValue4);
// scenario 2
valid = valid || (bValue1 && bValue2 && bValue3 && !bValue4);
// scenario 3
valid = valid || (bValue1 && !bValue2 && !bValue3 && !bValue4);
Франсуа Геген
источник
2

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


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

public class Options {
  public const bool A = 2; // 0001
  public const bool B = 4; // 0010
  public const bool C = 16;// 0100
  public const bool D = 32;// 1000
//public const bool N = 2^n; (up to n=32)
}

...

public isScenario3(int options) {
  int s3 = Options.A | Options.B | Options.C;
  // for true if only s3 options are set
  return options == s3;
  // for true if s3 options are set
  // return options & s3 == s3
}

Это делает выражение сценария таким же простым, как перечисление того, что является его частью, позволяет использовать оператор switch для перехода к нужному условию и сбивать с толку коллег-разработчиков, которые не видели этого раньше. (C # RegexOptions использует этот шаблон для установки флагов, я не знаю, есть ли пример библиотеки C ++)

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

ifНекоторым людям будет легче читать вложенные s. Вот моя версия

bool check(int bValue1, int bValue2, int bValue3, int bValue4)
{
  if (bValue1)
  {
    if (bValue2)
    {
      // scenario 1-2
      return bValue3;
    }
    else
    {
      // scenario 3
      return !bValue3 && !bValue4;
    }
  }

  return false;
}
sardok
источник
Лично я обычно избегаю вложенных операторов if, если это возможно. Хотя этот случай приятный и читаемый, как только добавляются новые возможности, вложение может стать очень трудным для чтения. Но если сценарии никогда не меняются, это определенно хорошее и удобочитаемое решение.
Dnomyar96 06
@ Dnomyar96 согласен. Лично я тоже избегаю вложенных «если». Иногда, если логика сложна, мне легче понять логику, разбив ее на части. Например, как только вы вводите bValue1блок, вы можете рассматривать все в нем как новую свежую страницу в своем умственном процессе. Бьюсь об заклад, подход к проблеме может быть очень личным или даже культурным.
sardok 06
1

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

В реальной жизни, когда мы находим такую ​​ситуацию:

         Scenario 1 | Scenario 2 | Scenario 3
bValue1: true       | true       | true
bValue2: true       | true       | false
bValue3: true       | true       | false
bValue4: true       | false      | false

Когда четыре состояния связаны таким точным шаблоном, мы имеем дело с конфигурацией некой «сущности» в нашей модели. .

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

Очевидно, что способ уменьшить сложность - это абстракция, а в C ++ предпочтительным инструментом является объектная парадигма .

Возникает вопрос: почему существует такая закономерность? Что это такое и что из себя представляет?

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

                0   1   2   3
Scenario 1:     T   T   T   T
Scenario 2:     T   T   T   F
Scenario 3:     T   F   F   F

На этом этапе у вас есть исходная конфигурация. как массив. Например, std::arrayесть оператор равенства:

В этот момент ваш синтаксис становится:

if( myarray == scenario1 ) {
  // arrays contents are the same

} 
else if ( myarray == scenario2 ) {
  // arrays contents are the same

} 

else if ( myarray == scenario3 ) {
  // arrays contents are the same

} 
else {
  // not the same

}

Как и ответ Джан Паоло, он короткий, ясный и легко проверяемый / отлаживаемый. В этом случае мы делегировали детали булевых выражений компилятору.

fralau
источник
1

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

Допустимые значения:

         Scenario 1 | Scenario 2 | Scenario 3
bValue1: true       | true       | true
bValue2: true       | true       | false
bValue3: true       | true       | false
bValue4: true       | false      | false

У вас явно есть три состояния (сценария). Было бы лучше смоделировать это и вывести логические свойства из этих состояний, а не наоборот.

enum State
{
    scenario1,
    scenario2,
    scenario3,
};

inline bool isValue1(State s)
{
    // (Well, this is kind of silly.  Do you really need this flag?)
    return true;
}

inline bool isValue2(State s)
{
    switch (s)
    {
        case scenario1:
        case scenario2:
            return true;
        case scenario3:
            return false;
    }
}

inline bool isValue3(State s)
{
    // (This is silly too.  Do you really need this flag?)
    return isValue2(s);
}

inline bool isValue4(State s)
{
    switch (s)
    {
        case scenario1:
            return true;
        case scenario2:
        case scenario3:
            return false;
    }
}

Это определенно больше кода, чем в ответе Джан Паоло , но в зависимости от вашей ситуации это может быть гораздо более удобным в обслуживании:

  • Существует центральный набор функций, которые можно изменить при добавлении дополнительных логических свойств или сценариев.
    • Для добавления свойств требуется добавить только одну функцию.
    • При добавлении сценария включение предупреждений компилятора о необработанных enumслучаях в switchоператорах перехватит средства получения свойств, которые не обрабатывают этот сценарий.
  • Если вам нужно динамически изменять логические свойства, вам не нужно везде повторно проверять их комбинации. Вместо переключения отдельных логических флагов (что может привести к недопустимым комбинациям флагов) у вас будет конечный автомат, который переходит от одного сценария к другому.

У этого подхода также есть побочное преимущество - он очень эффективен.

jamesdlin
источник
0

Мои 2 цента: объявить переменную сумму (целое число), чтобы

if(bValue1)
{
  sum=sum+1;
}
if(bValue2)
{
  sum=sum+2;
}
if(bValue3)
{
  sum=sum+4;
}
if(bValue4)
{
  sum=sum+8;
}

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

SCdev
источник
0

Принятый ответ хорош, если у вас всего 3 случая и логика для каждого проста.

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

Вы создаете, BaseValidatorкоторый содержит ссылку на, BaseValidatorа также метод validateи метод для вызова проверки на указанном валидаторе.

class BaseValidator {
    BaseValidator* nextValidator;

    public:
    BaseValidator() {
        nextValidator = 0;
    }

    void link(BaseValidator validator) {
        if (nextValidator) {
            nextValidator->link(validator);
        } else {
            nextValidator = validator;
        }
    }

    bool callLinkedValidator(bool v1, bool v2, bool v3, bool v4) {
        if (nextValidator) {
            return nextValidator->validate(v1, v2, v3, v4);
        }

        return false;
    }

    virtual bool validate(bool v1, bool v2, bool v3, bool v4) {
        return false;
    }
}

Затем вы создаете несколько подклассов, которые наследуются от метода BaseValidator, переопределяя validateего логикой, необходимой для каждого валидатора.

class Validator1: public BaseValidator {
    public:
    bool validate(bool v1, bool v2, bool v3, bool v4) {
        if (v1 && v2 && v3 && v4) {
            return true;
        }

        return nextValidator->callLinkedValidator(v1, v2, v3, v4);
    }
}

Затем использовать его просто: создайте экземпляр каждого из ваших валидаторов и установите каждый из них в качестве корня для других:

Validator1 firstValidator = new Validator1();
Validator2 secondValidator = new Validator2();
Validator3 thirdValidator = new Validator3();
firstValidator.link(secondValidator);
firstValidator.link(thirdValidator);
if (firstValidator.validate(value1, value2, value3, value4)) { ... }

По сути, каждый случай проверки имеет свой собственный класс, который отвечает за (а) определение того, соответствует ли проверка этому случаю, и (б) отправка валидации кому-то еще в цепочке, если это не так.

Обратите внимание, что я не знаком с C ++. Я попытался сопоставить синтаксис с некоторыми примерами, которые нашел в Интернете, но если это не сработает, относитесь к нему как к псевдокоду. У меня также есть полный рабочий пример Python ниже, который при желании можно использовать в качестве основы.

class BaseValidator:
    def __init__(self):
        self.nextValidator = 0

    def link(self, validator):
        if (self.nextValidator):
            self.nextValidator.link(validator)
        else:
            self.nextValidator = validator

    def callLinkedValidator(self, v1, v2, v3, v4):
        if (self.nextValidator):
            return self.nextValidator.validate(v1, v2, v3, v4)

        return False

    def validate(self, v1, v2, v3, v4):
        return False

class Validator1(BaseValidator):
    def validate(self, v1, v2, v3, v4):
        if (v1 and v2 and v3 and v4):
            return True
        return self.callLinkedValidator(v1, v2, v3, v4)

class Validator2(BaseValidator):
    def validate(self, v1, v2, v3, v4):
        if (v1 and v2 and v3 and not v4):
            return True
        return self.callLinkedValidator(v1, v2, v3, v4)

class Validator3(BaseValidator):
    def validate(self, v1, v2, v3, v4):
        if (v1 and not v2 and not v3 and not v4):
            return True
        return self.callLinkedValidator(v1, v2, v3, v4)

firstValidator = Validator1()
secondValidator = Validator2()
thirdValidator = Validator3()
firstValidator.link(secondValidator)
firstValidator.link(thirdValidator)
print(firstValidator.validate(False, False, True, False))

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

Джим Каллен
источник
-2

Простой подход - найти ответ, который вы считаете приемлемым.

Да = (boolean1 && boolean2 && boolean3 && boolean4) + + ...

Теперь, если возможно, упростите уравнение с помощью булевой алгебры.

как и в этом случае, допустимые1 и 2 объединяются в (boolean1 && boolean2 && boolean3).

Отсюда окончательный ответ:

(boolean1 && boolean2 && boolean3) || 
((boolean1 && !boolean2 && !boolean3 && !boolean4)
Rupesh
источник
-3

использовать битовое поле :

unoin {
  struct {
    bool b1: 1;
    bool b2: 1;
    bool b3: 1;
    bool b4: 1;
  } b;
  int i;
} u;

// set:
u.b.b1=true;
...

// test
if (u.i == 0x0f) {...}
if (u.i == 0x0e) {...}
if (u.i == 0x08) {...}

PS :

Это большая жалость для CPP. Но UB меня не беспокоит, проверьте его на http://coliru.stacked-crooked.com/a/2b556abfc28574a1 .

hedzr
источник
2
Это вызывает UB из-за доступа к неактивному полю объединения.
HolyBlackCat
Формально это UB в C ++, вы не можете установить один член объединения и читать из другого. Технически было бы лучше реализовать шаблонные геттеры \ сеттеры для битов целого значения.
Swift - Пятничный пирог
Я думаю, что поведение изменилось бы на «Определенное реализацией», если бы можно было преобразовать адрес объединения в unsigned char*, хотя я думаю, что простое использование чего-то вроде ((((flag4 <<1) | flag3) << 1) | flag2) << 1) | flag1, вероятно, было бы более эффективным.
supercat 04