Is this multi-line string too complex?

Question

I check the input of the form and try to prompt the user for the incorrect input (s) based on the combination of controls used.

For example, I have 2 combo boxes and 3 text fields. Two combo boxes should always have a value different from the first (default) value, but one of three or two of three or all text fields can be filled in to make the form valid.

In one of these scenarios, I have a 6 line instruction to make the test easy to read:

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 } 

I have 2 questions:

• If this type of if statement can be avoided at all costs?

• Would it be better to apply this test with another method? (This would be a good choice, as this check will be performed in more than one scenario)

Thanks for your details!

Answer 1

In that case, I find that this helps move some logic from the if statement to the more meaningful booleans names. For instance.

 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 } 

Obviously, name combos and text fields to reflect their actual content. When someone has to work their way through logic after several months in a row, they will be grateful to you.

Answer 2

• a) No need to be avoided at all costs. The code is working. But this is certainly messy, confusing, and I would say that it can be difficult to maintain.
• b) Yes. Give him the appropriate name so that the code reader knows what is going on there.

Answer 3

I personally would not have a big problem with such code. (Your last set of parentheses seems unnecessary.)

Generally, I would like to simplify my if statements. But all your conditions are simple. If you really need to verify that many tests, then I would save it as it is.

Answer 4

This is not very readable yes. But you can shorten it:

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

can also be written as:

 ComboBoxA.SelectedValue.ToString()!=DEFAULT_COMBO_A_CHOICE 

I assume that DEFAULT_COMBO_A_CHOICE already has the string ToString si superflous.

also parenthese around

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

Not needed.

Answer 5

IMO, such conditions should be avoided (although not at all costs). They are very difficult to read.

There are several ways to do this.

Try and group the conditions according to the behavior that they represent. for instance

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

This way you can also avoid duplicating conditions in your form.

Secondly, you can use Boolean Algebra to simplify the expression and

Use the extraction method to move conditions that are difficult to read in functions to avoid duplication and make them more readable.

For example, Condition

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

can be allocated to a function

 private bool IsDefaultA() { return ... }