Есть ли лучший способ структурировать это утверждение if

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

У меня есть 6 элементов управления на веб-странице.

if (printer_make_1.Text != "" && printer_model_1.Text != "" && printer_make_2.Text != "" && printer_model_2.Text != "" && printer_make_3.Text != "" && printer_model_3.Text != "") 
{ 
    // Do something 
}

Что является лучшим/наиболее эффективным способом для этого?

источник

2012-04-27 James Wilson

Ваш код прекрасно читается и почти наверняка правильно. Есть ли преимущество в бизнесе для изменения читаемого, правильного кода? –

, если мне нужно использовать инструкцию if, как это более одного раза, я определенно превращаю ее в метод, как предлагали другие, в противном случае я оставил бы его как есть – peroija

@EricLippert без преимущества, но в дополнение к моим собственным знаниям. Я определенно узнал немало новых способов сделать это, моими двумя фаворитами до сих пор являются опция метода и опция класса. –

Реструктуризация начинается с данных: избежать printer_make_1, printer_make_2 ...

class PrinterData 
{ 
    public string Make { get; set; } 
    public string Model { get; set; } 
} 

PrinterData[] printers = new PrinterData[3]; //or use a List<> 

printers[0] = new PrinterData { Make = "PH", Model = "1A" }; 
... 

if (printers.All(p => ! (p.Make == "" || p.Model == ""))) 
    ...

источник

2012-04-27 15:51:37

Если я позволю им заполнить три разных типа принтера/моделей, я бы просто создал еще 4 свойства? Я довольно новичок в asp.net. –

@Henk: Если вы являетесь его классом, вы можете использовать свойство 'HasValue' для удобства чтения, например' public bool HasValue {get {return! (Make.Length == 0 || Model.Length == 0); }} ' – Patrick

Нет принтеров - это список. Я сделаю это массивом, это легче понять. –

if(new[] { printer_make_1, printer_model_1 ...}.All(l => l.Text != string.Empty) 
{ 
    //do something 
}

Вы можете разбить его на более читаемым:

var labels = new[] { printer_make_1, printer_model_1 ... }; 
if(labels.All(l => l.Text != string.Empty)) 
{ 
    //do something 
}

источник

2012-04-27 15:47:04 Lee

Ха-ха, это вряд ли читаемо ... ИМХО. Конечно, это трюк. – Yuck

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

Менее читаемый * и * менее эффективный. –

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

public Boolean AllControlsHaveAValue() { 
    return (printer_make_1.Text != "" 
     && printer_model_1.Text != "" 
     && printer_make_2.Text != "" 
     && printer_model_2.Text != "" 
     && printer_make_3.Text != "" 
     && printer_model_3.Text != ""); 
}

Тогда просто спросить:

if (AllControlsHaveAValue()) { 
    // do something 
}

источник

2012-04-27 15:49:23 Yuck

+1 Проблема не обязательно в структуре if или необходимости писать ее по-другому, иногда просто имеет смысл обернуть ее в имя метода, которое можно прочитать. Напоминает мне о бескомпромиссном программировании. –

@AdamHouldsworth Mmmm ... commentless code :) Я думаю, что Spolsky написал замечательную статью об этом некоторое время назад. – Yuck

@Yuck: есть это: http://www.codinghorror.com/blog/2008/07/coding-without-comments.html – Patrick

Обычно я положил этот тест в метод и называют это сделать, если легче читать

private boolean AreAllPrinterFieldsFilled() 
{ 
    return (printer_make_1.Text != "" 
     && printer_model_1.Text != "" 
     && printer_make_2.Text != "" 
     && printer_model_2.Text != "" 
     && printer_make_3.Text != "" 
     && printer_model_3.Text != ""); 
}

Тогда в случае, если:

if (AreAllPrinterFieldsFilled) 
{ 
    // Do something 
}

источник

2012-04-27 15:49:40

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

я бы, вероятно, принять этот подход:

string makeText = String.Concat(printer_make_1.Text, printer_make_2.Text, printer_make_3.Text); 
string modelText = String.Concat(printer_model_1.Text, printer_model_2.Text, printer_model_3.Text); 

if (makeText.Length != 0 && modelText.Length != 0) 
{ 
    // Do something 
}

источник

2012-04-27 15:49:41

Зачем создавать несколько новых строк только для проверки значений нескольких существующих? –

@ChrisFarmer, создание нескольких строк тривиально. Опять же, есть ** много способов преодолеть это (как я уже говорил). Для меня это наиболее читаемо/понятно. Я не думаю, что создание нескольких строк вызовет любые проблемы с производительностью :) –

Я не оспариваю влияние производительности. Наверное, я просто сочувствую комментарию Эрика Липперта на вопрос, подразумевающий, что если он не сломался, не исправляйте его. –

if (!Enumerable.Range(1, 3) 
    .Any(i => ((TextBox)FindControl("printer_make_" + i)).Text == "" || 
     ((TextBox)FindControl("printer_model_" + i)).Text == "") {...}

Это позволяет впоследствии расширить количество принтера и моделей, но не так сильно типизированных.

источник

2012-04-27 15:50:44

private bool CheckAllEmpty(params TextBox[] textBoxes) 
{ 
    return textBoxes.All(tb => tb.Text != null); 
} 

private void Foo() 
{ 
    ... 

    if (CheckAllEmpty(tb1, tb2, tb3)) 
    { 
     mbox("tb1, tb2 and tb3 are all empty"); 
    } 
    else 
    { 
     mbox("tb1, tb2 or tb3 isn't empty"); 
    } 

    ... 
}

источник

2012-04-27 15:57:00 SimpleVar

Есть ли лучший способ структурировать это утверждение if

ответ

Смежные вопросы