I have a condition like this one:
if (string.IsNullOrEmpty(filename) || size != "Large" || size != "Medium" || size != "Small")
Probability in future I will have to manage more size
in the if
statement.
I would like to know if exist a more manageable and readable way to write this condition.
Please provide a real exampl开发者_运维问答e thanks for your time on this.
You can keep a hashset 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
}
// you could externalize and manage this list somewhere else
var sizes = new[] { "Large", "Medium", "Small" };
if (string.IsNullOrEmpty(filename) || !sizes.Contains(size))
{
...
}
Put the sizes in a collection of some sort, and use "Contains". See MSDN for an example: http://msdn.microsoft.com/en-us/library/bb352880.aspx
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 even cleaner code, encapsulate the checking of size into it's own method and amend that method when required:
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())
}
If you have more than one thing to do as a result of the test, writing it out as a switch/case might be more readable.
Otherwise, if you have loads of those values, it might be nicer to keep the list in some dictionary - let's say notHandledSizes
. Assign "Large" => true, "Medium" => true, ... and just check if size
is existing and true in that dictionary.
Option 1:
Write a small function that returns a bool and that only contains the size tests and use that in your 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 to test against and use 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.
Below line breaks makes it more readable.
if (string.IsNullOrEmpty(filename) ||
size != "Large" ||
size != "Medium" ||
size != "Small")
Design Tip
If many objects involve with your long if condition, it's good to write small properties/methods that return true/false in those Classes.
if (string.IsNullOrEmpty(filename) || object.IsProperSize)
Sometimes Enum Flags Attribute also helps such case. Look at here for a good explanation.
What you have done looks the most direct way of doing it. Any modification will simply be moving the mess elsewhere. If you do not need to reuse this code anywhere else, I would leave it the way it is.
精彩评论