Improved syntax for If statement

Question

I have a condition like this:

if (string.IsNullOrEmpty(filename) || size != "Large" || size != "Medium" || size != "Small") 

Probability in the future I will have to control more size in the if .

I would like to know if there is a more manageable and readable way of writing this condition.

Please give a real example, thanks for your time.

Answer 1

You can save a hash of words and check:

 HashSet<string> filterWords = new HashSet<string>(); // Put all words in the hash set if (filterWords.contains(size)) { // Do what ever you need } 

Answer 2

 // you could externalize and manage this list somewhere else var sizes = new[] { "Large", "Medium", "Small" }; if (string.IsNullOrEmpty(filename) || !sizes.Contains(size)) { ... } 

Answer 3

Put the sizes in some collection and use "Contains". See MSDN for an example: http://msdn.microsoft.com/en-us/library/bb352880.aspx

Answer 4

Option 1:

Write a small function that returns bool and which contains only size tests and uses them in if .

 if (string.IsNullOrEmpty(filename) || GoodSize(size)) { //... } private bool GoodSize(string size) { return size != "Large" || size != "Medium" || size != "Small"; } 

Option 2:

Create a list of sizes for testing and using Contains :

 var goodSizes = new[] { "Large", "Medium", "Small" }; if (string.IsNullOrEmpty(filename) || !goodSizes.Contains(size)) { //... } 

And you can combine both options for better clarity and encapsulation.

Answer 5

If they are going to change, perhaps a static list is better:

 private static List<string> Sizes = new List<string> { "large", "medium", "small" }; if (string.IsNullOrEmpty(filename) || Sizes.Contains(size.ToLower())) { } 

For cleaner code, encapsulate the size check in your own method and modify this method if necessary:

 if (MeetsSizeRequirementsOrIsNull(filename, size)) { } private static bool MeetsSizeRequirementsOrIsNull(string filename, string size) { List<string> sizes = new List<string>() { "..." }; return string.IsNullOrEmpty(filename) || sizes.Contains(size.ToLower()) } 

Answer 6

If you have a few things to do as a result of the test, writing it as a switch / case may be more understandable.

Otherwise, if you have values ​​for these values, it would be better to save the list in some dictionary - say notHandledSizes . Assign "Large" => true, "Medium" => true, ... and just check if size and true exist in this dictionary.

Answer 7

What you did looks like the most direct way to do it. Any modification will simply move the mess elsewhere. If you do not need to reuse this code elsewhere, I would leave it as it is.

Answer 8

Below line breaks make it more readable.

  if (string.IsNullOrEmpty(filename) || size != "Large" || size != "Medium" || size != "Small") 

Design tip

If many objects are associated with your long if condition, it is useful to write small properties / methods that return true / false in these classes.

 if (string.IsNullOrEmpty(filename) || object.IsProperSize) 

Sometimes the Enum attribute attribute also helps in this case. Look here for a good explanation.