Most readable way to write a simple conditional check

Question

What would be the most readable / best way to write multiple conditional checks like the one below?

Two possibilities that I could think of (this is Java, but this language doesn’t really matter):

Option 1:

boolean c1 = passwordField.getPassword().length > 0; boolean c2 = !stationIDTextField.getText().trim().isEmpty(); boolean c3 = !userNameTextField.getText().trim().isEmpty(); if (c1 && c2 && c3) { okButton.setEnabled(true); } 

Option 2:

  if (passwordField.getPassword().length > 0 && !stationIDTextField.getText().trim().isEmpty() && !userNameTextField.getText().trim().isEmpty() { okButton.setEnabled(true); } 

What I don't like about option 2 is that the line wraps around and then the indentation becomes a pain. What I don't like about option 1 is that it creates variables for nothing and requires a search in two places.

And what do you think? Any other options?

Answer 1

 if (HasPassword() && HasStation() && HasUserName()) okButton.setEnabled(true); bool HasPassword() { return passwordField.getPassword().length > 0; } 

and etc.

Answer 2

Please note that option 1 does not allow short circuits. That is, you calculate the value of all conditional numbers before evaluating the result of the first.

Answer 3

I would change option 1 so that you use variable names that really matter. That is, change the name "c2" to something like "stationIDIsEmpty" (and move NOT to the conditional). Thus, the conditional is readable without looking back and forth for each variable.

So, my code will probably look like this:

 boolean enteredPassword = passwordField.getPassword().length > 0; boolean stationIDIsEmpty = stationIDTextField.getText().trim().isEmpty(); boolean userNameIsEmpty = userNameTextField.getText().trim().isEmpty(); if (enteredPassword && !stationIDIsEmpty && !userNameIsEmpty) { okButton.setEnabled(true); } 

Answer 4

I voted for Chris Brandsma's answer.

But just wanted to mention the main problem that I have in Option 1, you lose the & & advantage. With one option, although I find it more readable, you handle comparisons when they might not be needed.

Answer 5

Personally, I like the second method, because I find that using this method can make the predicate of conventions understandable. That is, using this method, which you did right, you can make the conditional understandable by "verablizing" (regardless of whether you really say that it does not matter).

That is, with your second option, it becomes clear that your conditional translation approximately corresponds to this: "If the password is longer than zero, AND stationIDTextField (is truncated) is NOT empty, AND usernameTextField (is truncated) NOT empty, then ..."

Answer 6

I prefer the following:

 if (passwordField.getPassword().length > 0 && ! stationIDTextField.getText().trim().isEmpty() && ! userNameTextField.getText().trim().isEmpty()) { okButton.setEnabled(true); } 

With this coding style, I accomplish two things:

• I can easily see that every additional if line is part of the condition due to && (or ||) at the beginning.
• I can easily see where the if statement ends due to {on the next line.

Answer 7

Option 1 is easy to apply refactoring Replace temp with the request . The reason is that someone can use the code between the variable, initializes and checks and changes the behavior of the code. Or, validation may be performed with outdated values ​​.. text fields were updated between initialization and validation.

So my attempt:

 if (GetPasswordLength() > 0 && FieldHelper.IsNotEmpty(stationIDTextField) && FieldHelper.IsNotEmpty(userNameTextField) { okButton.setEnabled(true); } 

FieldHelper is a class with public static methods (also called Utility / Static class in C #)