Refactoring big if else code
I have this really long if else code
if (errorNumbers.Length == 6)
{
if (errorNumbers.Substring(0,4).Equals("1101") || errorNumbers.Substring(0,4).Equals("2121"))
{
retStr = "AutoRepickAfterPickError";
}
else if (errorNumbers.Substring(0, 4).Equals("开发者_JAVA百科1301") || errorNumbers.Substring(0, 4).Equals("2321"))
{
retStr = "AutoRepickAfterLAlignError";
}
else if (errorNumbers.Substring(0, 4).Equals("1401") || errorNumbers.Substring(0, 4).Equals("2221"))
{
retStr = "AutoRepickAfterCAlignError";
}
else if (errorNumbers.Substring(0, 4).Equals("1501") || errorNumbers.Substring(0, 4).Equals("2041"))
{
retStr = "AutoRepicksAfterManualRecovery";
}
etc.....
How I can rewrite it to something more nice . Trying to learn here something new , and I am in .net 2.0. Thanks for help.
I´d map the error codes in a dictionary like this:
string errorCode = errorNumbers.Substring(0, 4);
Dictionary<string, string> map = new Dictionary<string,string>();
map.Add("1501", "AutoRepicksAfterManualRecovery");
map.Add("2041", "AutoRepicksAfterManualRecovery");
map.Add("1401", "AutoRepickAfterCAlignError");
map.Add("2221", "AutoRepickAfterCAlignError");
map.Add("1301", "AutoRepickAfterPickError");
map.Add("2121", "AutoRepickAfterPickError");
// etc
if(!map.ContainsKey(errorCode))
throw new Exception("Invalid error code");
return map[errorCode];
That should be pretty straightforward
if (errorNumbers.Length == 6)
{
string errNo = errorNumbers.Substring(0, 4);
switch (errNo)
{
case "1101":
case "2121":
retStr = "AutoRepickAfterPickError";
break;
case "1301":
case "2321":
retStr = "AutoRepickAfterLAlignError";
break;
case "1401":
case "2221":
retStr = "AutoRepickAfterCAlignError";
break;
case "1501":
case "2041":
retStr = "AutoRepicksAfterManualRecovery";
break;
}
}
Also note that unlike in Java, in C# you can compare strings with ==
.
"123" == "456"
does the same thing as "123".Equals("456")
.
First off, DRY Principle - Don't Repeat Yourself. Assign the result of errorNumbers.Substring(0, 4)
to a local variable:
string subNumbers = errorNumbers.SubString(0, 4);
if (subNumbers == "1401") // ...
Support for specific value cases (not ranges):
You can use a switch statement with strings:
switch(subNumbers)
{
case "1401":
case "2221":
retStr = "AutoRepickAfterCAlignError";
break;
case "1501":
case "2041":
retStr = "AutoRepicksAfterManualRecovery";
break;
// etc
}
An alternative to a switch statement is creating a dictionary:
var selections = new Dictionary<string, string>()
{
{ "1401", "AutoRepickAfterCAlignError" },
{ "2221", "AutoRepickAfterCAlignError" },
{ "1501", "AutoRepicksAfterManualRecovery" },
{ "2041", "AutoRepicksAfterManualRecovery" },
// etc
};
if (selections.ContainsKey(subNumbers))
retStr = selections[subNumbers];
Ranges:
If you need to support ranges, you're basically going to have to stick with the if
/else
blocks, though. There are other options, but they tend to be too convoluted if you only have so many of these types of if
/else
blocks in your code.
You should initialize a dictionary with refferencies betwen errorCode and errorMessage
Dictionary<int, string> errorsCache = new Dictionary<int, string>()
{
{1101,"AutoRepickAfterPickError"},
{2121,"AutoRepickAfterPickError"},
{1401,"AutoRepickAfterCAlignError"},
...
}
You can define GetErrorDescription method
public string GetErrorDescription(string errorNumbers)
{
string retStr;
if (errorNumbers.Length == 6)
{
int errorCode;
if(int.TryParse(errorNumbers.Substring(0,4), out errorCode))
{
errorsCache.TryGetValue(errorCode, out retStr);
}
}
return retString;
// or you can return empty string instead of null
return retString ?? "";
}
First, store errorNumbers.Substring(0,4)
in a local variable before the long if/else.
Then, you can eliminate the whole if/else using a map / dictionary from the expected error numbers to the appropriate return strings.
You could try this:
if (errorNumbers.Length == 6)
{
//Makes it easier to change error codes, as theyre all in one place
string[] AutoRepickAfterPickError = { "1101", "2121"};
string[] AutoRepickAfterLAlignError = { "1301", "2321"};
string[] AutoRepickAfterCAlignError = { "1401", "2221"};
string[] AutoRepicksAfterManualRecovery = { "1501", "2041"};
string subString = errorNumbers.Substring(0,4);
if (AutoRepickAfterPickError.Contains(subString));
{
retStr = "AutoRepickAfterPickError";
}
else if (AutoRepickAfterLAlignError.Contains(subString))
{
retStr = "AutoRepickAfterLAlignError";
}
else if (AutoRepickAfterCAlignError.Contains(subString))
{
retStr = "AutoRepickAfterCAlignError";
}
else if (AutoRepicksAfterManualRecovery.Contains(subString))
{
retStr = "AutoRepicksAfterManualRecovery";
}
}
What about Select-Case? :D
errorNumber = errorNumbers.Substring(0,4)
Select Case errorNumber
Case 1101,2121
retStr = "AutoRepickAfterPickError";
Case 1301,2321
retStr = "AutoRepickAfterLAlignError";
...
End Select
精彩评论