Является ли эта многострочная инструкция слишком сложной?

Question

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

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

В одном такомСценарий У меня есть инструкция из 6 строк, чтобы попытаться сделать тест легко читаемым:

if ((!String.Equals(ComboBoxA.SelectedValue.ToString(), DEFAULT_COMBO_A_CHOICE.ToString())
    && !String.IsNullOrEmpty(TextBoxA.Text)
    && !String.Equals(ComboBoxB.SelectedValue.ToString(), DEFAULT_COMBO_B_CHOICE.ToString()))        
    ||
    (!String.IsNullOrEmpty(TextBoxB.Text)
    || !String.IsNullOrEmpty(TextBoxC.Text)))
{
    //Do Some Validation
}

У меня есть 2 вопроса:

1. Если этот тип если заявление следует избегать любой ценой?

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

Спасибо за ваш вклад (ы)!

Answer 1

В таком случае я считаю, что это помогает переместить некоторую логику из оператора if в более логически названные логические значения. Например.

bool comboBoxASelected = !String.Equals(ComboBoxA.SelectedValue.ToString(), DEFAULT_COMBO_A_CHOICE.ToString());
bool comboBSelected = !String.Equals(ComboBoxB.SelectedValue.ToString(), DEFAULT_COMBO_B_CHOICE.ToString());
bool textBoxAHasContent = !String.IsNullOrEmpty(TextBoxA.Text);
bool textBoxBHasContent = !String.IsNullOrEmpty(TextBoxB.Text);
bool textBoxCHasContent = !String.IsNullOrEmpty(TextBoxC.Text);

bool primaryInformationEntered = comboBoxASelected && textBoxAHasContent && comboBSelected;
bool alternativeInformationEntered = textBoxBHasContent || textBoxCHasContent;

if (primaryInformationEntered || alternativeInformationEntered)
{
    //Do Some Validation
}

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

Answer 2

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

Answer 3

У меня лично не было бы большой проблемы с таким кодом.(Ваш последний набор скобок кажется ненужным.)

Как правило, я бы хотел упростить свои операторы if.Но все ваши условия просты.Если вам действительно нужно протестировать столько тестов, я бы оставил все как есть.

Answer 4

ИМО таких условий следует избегать (хотя и не любой ценой).Их очень трудно прочитать в сопровождении.

Есть несколько способов сделать это

Попробуйте сгруппировать условия в соответствии с поведением, которое они представляют.Например,

if (OrderDetailsSelected() && ShippingAddressProvided() )  
{

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

Во-вторых, вы можете использовать Булева алгебра , чтобы упростить выражениеи

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

Например,Условие

String.Equals(ComboBoxB.SelectedValue.ToString(), DEFAULT_COMBO_B_CHOICE.ToString())

можно извлечь в функцию

private bool IsDefaultA() { return ... }

Answer 5

Это не очень читабельно, да. Но вы можете сократить его:

!String.Equals(ComboBoxA.SelectedValue.ToString(), DEFAULT_COMBO_A_CHOICE.ToString()

также может быть записано как:

ComboBoxA.SelectedValue.ToString()!=DEFAULT_COMBO_A_CHOICE

Я полагаю, что DEFAULT_COMBO_A_CHOICE уже содержит строку ToString si излишнюю.

также скобки вокруг

(!String.IsNullOrEmpty(TextBoxB.Text)
|| !String.IsNullOrEmpty(TextBoxC.Text))

не нужны.