Как мне переписать очень большой составной оператор if в C#?
В моем коде C# у меня есть выражение if, которое началось достаточно невинно:
if((something == -1) && (somethingelse == -1) && (etc == -1)) {
// ...
}
Это растет. Я думаю, что сейчас должно быть 20 пунктов.
Как я должен справиться с этим?
18 ответов
Используйте ворота там, где это возможно.
оператор if
if(bailIfIEqualZero != 0 &&
!string.IsNullOrEmpty(shouldNeverBeEmpty) &&
betterNotBeNull != null &&
!betterNotBeNull.RunAwayIfTrue &&
//yadda
реорганизованная версия
if(bailIfIEqualZero == 0)
return;
if(string.IsNullOrEmpty(shouldNeverBeEmpty))
return;
if(betterNotBeNull == null || betterNotBeNull.RunAwayIfTrue)
return;
//yadda
Разложите его в функцию и сделайте каждое условие охранной оговоркой:
int maybe_do_something(...) {
if(something != -1)
return 0;
if(somethingelse != -1)
return 0;
if(etc != -1)
return 0;
do_something();
return 1;
}
Предполагая, что все эти условия действительно необходимы, вы можете объединить условия в один или несколько uber-логических значений или вызовов функций для улучшения читабельности.
Например,
bool TeamAIsGoForLaunch = BobSaysGo && BillSaysGo;
bool TeamBIsGoForLaunch = JillSaysGo && JackSaysGo;
if (TeamAIsGoForLaunch && TeamBIsGoForLaunch && TeamC.isGoForLaunch())
Одна вещь, чтобы рассмотреть, почему у вас так много предложений. Подобно тому, как операторы SWITCH часто указывают, что вы должны перемещать варианты выбора в подклассы, большая сложная цепочка операторов IF может указывать на то, что вы объединяете слишком много концепций (и, следовательно, решений) в одном месте.
В качестве примера я буду использовать пример расчета комиссии. В одной из построенных мною комиссионных ставок зависит от суммы комиссионных, выплачиваемых оптовику, который передает часть розничному продавцу. Сумма, передаваемая от оптовика к розничному продавцу, зависит от конкретного соглашения между розничным торговцем и оптовым продавцом (на основании договора). Количество, которое оптовик получает, также зависит от продуктовой линейки, конкретного продукта и количества проданного продукта.
Помимо этого, существуют дополнительные правила "исключений", основанные на состоянии клиента, конкретных настройках продукта и т. Д. Все это может быть разработано в сложной цепочке операторов IF, но мы вместо этого сделали данные приложения управляемыми.
Поместив всю эту информацию в таблицы, мы смогли выполнить правила данных. Сначала срабатывают общие правила оптовика, а затем срабатывают любые правила переопределения. Затем, имея базовую комиссию оптовика, мы заставим оптовика распространять общие правила розничного предприятия, а затем исключения из них.
Это превратило огромный шарик логики в простой четырехэтапный процесс, при котором каждый шаг просто выполнял запрос к базе данных, чтобы найти правильное правило для применения.
Это, конечно, может не применяться в вашей ситуации, но часто такой большой комплекс действительно означает, что либо недостаточно классов, разделяющих ответственность (другим решением нашей проблемы могли быть фабричные классы, которые содержали конкретные правила и переопределения), или функциональность должна быть управляемой данными.
Рефакторинг это к функции.
bool Check()
{
return (something == -1) && (somethingelse == -1) && (etc == -1);
}
Кроме того, вы можете создать более читаемый код / логику в вашей функции проверки.
Есть много способов справиться с этим, но позвольте мне выбрать несколько.
Во-первых, это тот случай, когда все критерии (все AND-критерии в вашем операторе if) + код, который нужно выполнить, если все они истинны, являются одноразовыми.
В этом случае используйте код, который у вас есть. Возможно, вы захотите сделать то, что несколько других уже предложили, переписать, чтобы использовать код типа Guard-clause.
Другими словами, вместо этого:
if (a && b && c && d && ......)
DoSomething();
... вы переписываете что-то похожее на это:
if (!a) return;
if (!b) return;
if (!c) return;
if (!d) return;
if (!...) return;
DoSomething();
Зачем? Потому что, как только вы начинаете вводить OR-критерии в микс, становится трудно читать код и выяснять, что произойдет. В приведенном выше коде вы разделяете критерии для каждого оператора AND (&&), и, таким образом, код становится проще для чтения. По сути, вы переписываете код, говоря "если и то, и то, или другое, и третье, или что-то другое, то делаете что-то" чтобы быть ", если это, затем выходите; если это другое; затем выходите; если какое-то другое дело, затем выход; если ничего из вышеперечисленного, сделать что-то ".
Однако во многих случаях у вас также есть случай повторного использования. Если некоторые из этих критериев появляются где-то еще, но код, который будет фактически выполняться (DoSomething), не будет таким же, то я бы снова пошел на то, что уже предложили другие. Перепишите критерии в методы, которые возвращают Boolean результат в зависимости от результата оценки критериев.
Например, что легче читать, это?
if (a && b && c && d && e && f && (h || i) && (j || k) || l)
или это:
if (CanAccessStream() && CanWriteToStream())
при условии, что все эти буквы можно разделить на эти два критерия.
В этом случае я бы взял некоторые критерии и включил в эти методы и выбрал подходящее название для критериев.
Третий вариант - это когда критерии отличаются в нескольких местах кода, но фактический код для выполнения одинаков.
В этом случае я бы переписал так, чтобы вы сгруппировали критерии вместе и разбили слои на слои так, чтобы вызов одного метода проверил некоторые критерии, а затем вызвал другой метод, который проверит некоторые другие критерии и т.д.
Например, вы можете написать это:
if (stream != null && buffer != null && inBuffer > 0 && stream.CanWrite)
stream.Write(buffer, 0, inBuffer);
else
throw new InvalidOperationException();
или вы могли бы написать это:
if (inBuffer > 0)
{
Debug.Assert(buffer != null);
WriteToStream(buffer, inBuffer);
}
...
private void WriteToStream(Byte[] buffer, Int32 count)
{
if (stream.CanWrite)
stream.Write(buffer, 0, count);
else
throw new InvalidOperationException();
}
Я бы сказал, что второй способ легче читать и более пригоден для повторного использования, чем первый.
Вы могли бы реорганизовать его как функцию и вернуть значение Enum, которое представляет истинный случай:
if(something != -1)
return MyEnum.Something;
if(somethingelse != -1)
return MyEnum.SomethingElse;
if(etc != -1)
return MyEnum.SomethingElseEntirely;
return MyEnum.None;
Выполните совокупные операции со списком ваших значений.
if (new[] { something, somethingelse, ... }.All(x => x == -1)) {
}
* Редактировать: Предоставление данных дополнительной строки:
var Data = new[] { something, somethingelse, ... };
if (Data.All(x => x == -1)) {
}
Вы также можете разделить штат на класс.
class mystate
{
int something;
int somethingelse;
int etc;
bool abletodostuff()
{
return (something == -1) && (somethingelse == -1) && (etc == -1);
}
}
Похоже, у вас есть 3 части информации, которые вместе представляют определенное состояние в вашем приложении. Вместо того, чтобы включать эти 3 части состояния, почему бы не создать значение, которое инкапсулирует их? Затем вы можете использовать иерархию объектов или делегата во время создания, чтобы связать действие, которое вы пытаетесь выполнить.
Я не знаю C#, но, кажется, включает в себя условный оператор. Если ваши условия короткие, вы можете заменить длинные цепочки if/elsif/else хорошими табличными структурами, например так:
return something == 0 ? 0
: somethingelse == -1 ? 1
: yetanotherthing > 2 ? 2
: 3; # default
Еще одна возможность, которую вы можете исследовать, - это использование составных выражений. Эти выражения (которые являются основой работы LINQ в.NET Framework) позволяют создавать деревья выражений на основе всех этих условий, а затем код бизнес-логики может просто работать с выражением верхнего уровня, чтобы получить результат "истина / ложь".,
Чтобы оценить выражения, вы можете использовать шаблон посетителя
Это позволяет легко составлять деревья условий. эти деревья могут быть сериализованы даже для того, чтобы позволить вам сохранить условия, при которых вы приняли решение. Здесь есть много возможностей.
Не так часто можно видеть столько предложений в одном "если". Обычно вы обнаруживаете, что вам нужно вложить "если", чтобы получить необходимую логику, когда вам нужно выполнить некоторую строку независимо от истинности некоторых условий. Я не говорю, вкладывайте их, если вам это не нужно, если все они должны быть проверены одновременно. Только если есть какая-то общая функциональность. Другим соображением является установка логической переменной с результатом некоторого набора этих условий, которые могут облегчить понимание. Если ваши переменные являются массивом или коллекцией, можете ли вы пройти через них? Вы проверяете их все против -1?
И как то так
Я объясню немного дальше. (И исправить глупые ошибки:S)
//Interface to be able to which classes are able to give a boolean result used in the if stuff
public interface IResultable
{
bool Result();
}
//A list that is IResultable itself it gathers the results of the IResultables inside.
public class ComparatorList<T> : List<T>, IResultable where T : IResultable
{
public bool Result()
{
bool retval = true;
foreach (T t in this)
{
if (!t.Result())
{
retval = false;
}
}
return retval;
}
}
//Or two bools
public class OrComparator : IResultable
{
bool left;
bool right;
public OrComparator(bool left, bool right)
{
this.left = left;
this.right = right;
}
#region IResultable Members
public bool Result()
{
return (left || right);
}
#endregion
}
// And two bools
public class AndComparator : IResultable
{
bool left;
bool right;
public AndComparator(bool left, bool right)
{
this.left = left;
this.right = right;
}
#region IResultable Members
public bool Result()
{
return (left && right);
}
#endregion
}
// compare two ints
public class IntIsComparator : IResultable
{
int left;
int right;
public IntIsComparator(int left, int right)
{
this.left = left;
this.right = right;
}
#region IResultable Members
public bool Result()
{
return (left == right);
}
#endregion
}
Много ли у вас там заявлений, это может быть круто:)
Таким образом, у нас есть такая структура для проведения множества сравнений. Я приведу небольшой пример реализации.
//list of ands
ComparatorList<AndComparator> ands = new ComparatorList<AndComparator>();
ands.Add(new AndComparator(true,true));
//list of ors
ComparatorList<OrComparator> ors = new ComparatorList<OrComparator>();
ors.Add(new OrComparator(false, true));
//list of intiss
ComparatorList<IntIsComparator> ints = new ComparatorList<IntIsComparator>();
ints.Add(new IntIsComparator(1, 1));
//list of all things :)
ComparatorList<IResultable> collected = new ComparatorList<IResultable>();
collected.Add(ands);
collected.Add(ors);
collected.Add(ints);
// if all things are as they must be :)
if (collected.Result())
{
//Evertything is as it schould be :)
}
Я делаю это так, как никто, кроме меня. Мне все равно, если код "выглядит хорошо". Если у меня есть>1 тест в этом условном выражении, это означает, что я склонен хотеть еще большего, или я могу захотеть прокомментировать некоторые из них, или я хочу упростить внесение таких изменений.
Так что я кодирую это так:
if (true
&& test_1
&& test_2
...
)
{
...
}
Это позволяет легко закомментировать тесты или добавить новые в виде однострочных изменений.
Но я буду первым, чтобы признать, это не претендует на то, чтобы быть симпатичным.
Хотя мне нравится решение Дарио (как я уже прокомментировал, я бы поместил его в булеву функцию, чтобы у меня не было нового в условной части if...), я не уверен, что не так с:
if((something == -1) &&
(somethingElse == -1) &&
(elseElse == -1) &&
...
)
Я думаю, что это, вероятно, намного легче читать, чем много ((A && B) || (C &&(D||E))), с которыми мне приходится иметь дело...
Если вам действительно нужно проверить все эти условия, вы не можете избавиться от них. Вы можете только рефакторинг и сделать его более читабельным, но логика остается той же.
Я весьма удивлен, что никто ничего не упомянул о редизайне вашего кода. Вы действительно нуждаетесь в 20 разных штатах? По моему опыту, многие состояния часто зависят от других состояний и, следовательно, часто логически избыточны для проверки.
Рефакторинг вашего кода может помочь вам лучше понять его и то, как состояния связаны друг с другом. Я бы начал здесь первым на вашем месте:)
Вы говорите, что смотрите на строки, как насчет чего-то подобного, о котором кто-то уже прокомментировал.
var items = new List<string>();
items.Add("string1");
items.Add("string2");
if (items.Contains("string2"))
{
// do something
}
Вы могли бы даже получить значения из какого-либо файла конфигурации, чтобы заполнить список.