Зачем мне отразить этот код как цикломатическую сложность 58
https://stackoverflow.com/questions/7814401
-
26-10-2019 - |
italiano
english
français
española
中国
日本の
العربية
Deutsch
한국어
Português
RussianВопрос
Я читал, что наличие CC 10 или меньше будет хорошо обслуживаемым кодом. Но метод, который я написал, имеет CC 58. Благодаря инструменту анализа кода VS 2010. Я считаю, что метод, который я написал, очень прост, читается и поддерживается, насколько я понимаю. Следовательно, я бы не предпочел рефакторировать код. Но так как CC выше, чем приемлемо, мне интересно, почему рефактор этого метода. Я учусь вещам, чтобы улучшить свой код, если у меня ошибка, пожалуйста, поправьте меня. Вот код.
private string MapBathRooms(string value)
{
double retValue = 0;
if (value == "1" || value == "One")
retValue = 1;
if (value == "OneAndHalf" || value == "1.5" || value == "1 1/2")
retValue = 1.5;
if (value == "2" || value == "Two")
retValue = 2;
if (value == "TwoAndHalf" || value == "2.5" || value == "2 1/2")
retValue = 2.5;
if (value == "3" || value == "Three")
retValue = 3;
if (value == "ThreeAndHalf" || value == "3.5" || value == "3 1/2")
retValue = 3.5;
if (value == "4" || value == "Four")
retValue = 4;
if (value == "FourAndHalf" || value == "4.5" || value == "4 1/2")
retValue = 4.5;
if (value == "5" || value == "Five" || value == "FourOrMore")
retValue = 5;
if (value == "FiveAndHalf" || value == "5.5" || value == "5 1/2")
retValue = 5.5;
if (value == "6" || value == "Six")
retValue = 6;
if (value == "SixAndHalf" || value == "6.5" || value == "6 1/2")
retValue = 6.5;
if (value == "7" || value == "Seven")
retValue = 7;
if (value == "SevenAndHalf" || value == "7.5" || value == "7 1/2")
retValue = 7.5;
if (value == "8" || value == "8+" || value == "Eight" || value == "SevenOrMore")
retValue = 8;
if (value == "EightAndHalf" || value == "8.5" || value == "8 1/2")
retValue = 8.5;
if (value == "9" || value == "Nine")
retValue = 9;
if (value == "NineAndHalf" || value == "9.5" || value == "9 1/2")
retValue = 9.5;
if(value == "10" || value == "Ten")
retValue = 10;
if (value == "TenAndHalf" || value == "10.5" || value == "10 1/2"
|| value == "10+" || value == "MoreThanTen" || value == "11")
retValue = 10.5;
if (retValue == 0)
return value;
return retValue.ToString();
}
Решение
Почему бы просто не иметь Dictionary<string, double>? Это сделает для много более простой код - вы отделили данные из кода поиска.
private static readonly Dictionary<string, double> BathRoomMap =
new Dictionary<string, double>
{
{ "1", 1 },
{ "One", 1 },
{ "OneAndHalf", 1.5 },
{ "1.5", 1.5 },
{ "1 1/2", 1.5 }
// etc
};
private static string MapBathRooms(string value)
{
double result;
if (!BathRoomMap.TryGetValue(value, out result))
{
return value; // Lookup failed
}
return result.ToString();
}
На самом деле, вы можете сделать это еще проще, избегая вызова Tostring - просто сделайте его Dictionary<string, string>:
private static readonly Dictionary<string, string> BathRoomMap =
new Dictionary<string, string>
{
// Note: I've removed situations where we'd return the
// same value anyway... no need to map "1" to "1" etc
{ "One", "1" },
{ "OneAndHalf", "1.5" },
{ "1 1/2", "1.5" }
// etc
};
private static string MapBathRooms(string value)
{
string result;
if (!BathRoomMap.TryGetValue(value, out result))
{
return value; // Lookup failed
}
return result;
}
Как говорит Chrisf, вы также можете прочитать это из файла или другого ресурса.
Преимущества этого:
- Это много Легче избежать ошибок и расширить, ИМО. Есть простое отображение 1: 1 от ввода к выводу, в отличие от логики, которая мог пойти не так
- Он отделяет данные от логики
- Это позволяет загружать данные из других мест, если это будет необходимо.
- Потому что инициализаторы сбора используют
Dictionary<,>.Add, если у вас есть дубликат ключа, вы получите исключение при инициализации типа, поэтому вы немедленно обнаружите ошибку.
Поместить это так - ты бы Когда-либо Рассмотрим рефакторинг из версии на основе словаря в версию «Много реального кода»? Я, конечно, не буду.
Если вы действительно, действительно хотите иметь все это в методе, вы всегда можете использовать оператор Switch:
private static string MapBathRooms(string value)
{
switch (value)
{
case "One":
return "1";
case "OneAndHalf":
case "1 1/2":
return "1.5";
...
default:
return value;
}
}
Я бы все еще использовал форму словаря самостоятельно ... но это имеет очень небольшое преимущество, что повторное обнаружение выдвинуто в компиляция-время.
Другие советы
Я согласен с другими плакатами об использовании словаря для картирования, но я также хочу указать, что такой код часто трудно найти ошибки. Например:
- Вы конвертируете «Fourormore» в 5, но «Morethanten» преобразуется в 10,5. Это кажется непоследовательным.
- Вы конвертируете «11» в 10,5, что также кажется несовместимым с остальной частью кода.
Общий алгоритм для преобразования может быть труднее писать изначально, но может легко сэкономить время в долгосрочной перспективе.
Чтобы ответить на то, почему, а не как:
Одна из причин - та, которую я упоминаю в своем комментарии на ответе Джона Скита, но использование словаря и внешнего ресурса позволяет изменить поведение вашего приложения без необходимости восстанавливать его каждый раз, когда требования меняются.
Другой - скорость исполнения. Ваш код должен проверить несколько десятков строк, чтобы найти результат - в то время как есть способы остановить выполнение, как только вы найдете совпадение, вам все равно придется проверить их все. Использование словаря даст вам линейное время доступа независимо от ввода.
Ага. Действительно очень обслуживается.
Попробуйте вместо этого:
// initialize this somewhere
IDictionary<string, string> mapping;
private string MapBathRooms(string value)
{
if (mapping.ContainsKey(value))
{
return mapping[value];
}
return value;
}
Сохранение этого в словаре должно сохранить CC только 2. Словарь может быть инициализирован, читая его из файла или другого ресурса.
CC (в значительной степени) количество потенциальных путей выполнения метода. Ваш CC для этого метода настолько высок, потому что вы не использовали структуру, подходящую для решения таких проблем (здесь: словарь). Использование соответствующих структур данных для решения задач сохраняет свой код щедро и повторно.
С принципом сухого (не повторяйте себя) вы можете заменить все это if Заявления с switch. Анкет Переключатель будет реализован с использованием хэш -таблицы, поэтому он также будет быстрее, чем все if заявления.
Вы можете удалить все случаи, когда вы ловите числовое представление числа, так как это обрабатывается запасным.
Я не вижу точки преобразования строки в число, а затем снова обратно на строку. Более трудно использовать буквальные строки (как они предварительно созданы), чем создание строки на лету. Кроме того, это эллиминает проблему культуры, где, например, ценность 9.5 приведет к строке "9,5" вместо "9.5" для некоторых культур.
private string MapBathRooms(string value) {
switch (value) {
case "One": value = "1"; break;
case "OneAndHalf":
case "1 1/2": value = "1.5"; break;
case "Two": value = "2"; break;
case "TwoAndHalf":
case "2 1/2": value = "2.5"; break;
case "Three": value = "3"; break;
case "ThreeAndHalf":
case "3 1/2": value = "3.5"; break;
case "Four": value = "4"; break;
case "FourAndHalf":
case "4 1/2": value = "4.5"; break;
case "Five":
case "FourOrMore": value = "5"; break;
case "FiveAndHalf":
case "5 1/2": value = "5.5"; break;
case "Six": value = "6"; break;
case "SixAndHalf":
case "6 1/2": value = "6.5"; break;
case "Seven": value = "7"; break;
case "SevenAndHalf":
case "7 1/2": value = "7.5"; break;
case "8+":
case "Eight":
case "SevenOrMore": value = "8"; break;
case "EightAndHalf":
case "8 1/2": value = "8.5"; break;
case "Nine": value = "9"; break;
case "NineAndHalf":
case "9 1/2": value = "9.5"; break;
case "Ten": value = "10"; break;
case "TenAndHalf":
case "10 1/2":
case "10+":
case "MoreThanTen":
case "11": value = "10.5"; break;
}
return value;
}
Обратите внимание, что я оставил случай ввода "11" приводит к возвращению "10.5". Анкет Я не уверен, что это ошибка, но это то, что делает оригинальный код.
К вашему общему вопросу, для других случаев, которые не могут быть рефакторированы, как предложено другими респондентами для этой конкретной функции, существует вариант CC, который считает оператор случая в качестве единой ветви на том основании, что он практически такой же, как линейные строки Код в простоте понимания (хотя не для тестового покрытия). Многие инструменты, которые измеряют один вариант, будут предлагать другой. Я предлагаю использовать вариант Case = 1 вместо этого или, а также тот, который вы используете.
