Метрики кода Visual Studio и индекс ремонтопригодности коммутатора
Как человек, который любит следовать лучшим практикам,
Если я запускаю метрики кода (щелкните правой кнопкой мыши имя проекта в обозревателе решений и выберите "Рассчитать метрики кода" - Visual Studio 2010) на:
public static string GetFormFactor(int number)
{
string formFactor = string.Empty;
switch (number)
{
case 1:
formFactor = "Other";
break;
case 2:
formFactor = "SIP";
break;
case 3:
formFactor = "DIP";
break;
case 4:
formFactor = "ZIP";
break;
case 5:
formFactor = "SOJ";
break;
}
return formFactor;
}
Это дает мне индекс ремонтопригодности 61
(конечно, это незначительно, если у вас есть только это, но если вы используете утилиту наподобие класса, философия которого делает подобные вещи, у вашего служебного класса будет намного хуже индекс поддерживаемости...)
Какое решение для этого?
7 ответов
Прежде всего: 61 считается поддерживаемым кодом. Для Индекса сопровождения 100 очень легко поддерживать код, а 0 трудно поддерживать.
- 0-9 = трудно поддерживать
- 10-19 = умеренный для поддержания
- 20-100 = хорошо поддерживать
Индекс ремонтопригодности основан на трех метриках кода: именно объем Халстеда, цикломатическая сложность и строки кода и основан на следующей формуле:
МАКС. (0,(171 - 5,2 * ln(объем Холстеда) - 0,23 * (цикломатическая сложность) - 16,2 * ln(строки кода))*100 / 171)
Фактически, в вашем примере основной причиной низкого значения индекса ремонтопригодности является цикломатическая сложность. Этот показатель рассчитывается на основе различных путей выполнения в коде. К сожалению, метрика не обязательно совпадает с "удобочитаемостью" кода.
Примеры, приведенные в вашем коде, приводят к очень низким значениям индекса (помните, чем ниже, тем сложнее поддерживать), но на самом деле их очень легко прочитать. Это часто встречается при использовании Cyclomatic Complexity для оценки кода.
Представьте себе код, который имеет блок переключателей для дней (пн-вс) плюс блок переключателей для месяцев (январь-декабрь). Этот код будет очень удобочитаемым и поддерживаемым, но приведет к огромной цикломатической сложности и, следовательно, к очень низкому индексу обслуживания в Visual Studio 2010.
Это хорошо известный факт о метрике, и его следует учитывать, если код оценивается на основе рисунков. Вместо того, чтобы смотреть на абсолютное число, цифры должны отслеживаться с течением времени, чтобы понимать их как индикатор изменения кода. Например, если индекс ремонтопригодности всегда был равен 100 и внезапно опустился до 10, следует проверить изменение, вызвавшее это.
Индекс ремонтопригодности может быть выше из-за отсутствия расширяемости в методе, который вы выбираете для своего решения.
Правильное решение (о котором говорил Марк Симпсон) - это то, которое можно расширить для использования нового форм-фактора без перекомпоновки кода - операторы switch / case в коде всегда являются признаком того, что дизайн ОО был забыт и его всегда нужно видеть как плохой кодовый запах.
Лично я бы реализовал...
interface IFormFactor
{
// some methods
}
class SipFormFactor : IFormFactor...
class DipFormFactor : IFormFactor...
Etc.
... и чтобы методы в интерфейсе обеспечивали желаемую функциональность - вы можете думать о ней [я думаю] как о похожей на шаблон команд GoF.
Таким образом, ваши методы более высокого уровня могут быть просто...
MyMethod(IFormFactor factor)
{
// some logic...
factor.DoSomething();
// some more logic...
}
... и вы можете прийти позже и ввести новый форм-фактор, не меняя этот код, как если бы вы использовали жестко закодированное предложение switch. Вы также обнаружите, что этот подход также легко подходит для TDD (вы должны в конечном итоге с этим, если вы делаете TDD должным образом), так как это легко для насмешки.
Я бы сделал это таким образом и забыл бы индекс Maintainability:
public static string GetFormFactor(int number)
{
switch (number)
{
case 1: return "Other";
case 2: return "SIP";
case 3: return "DIP";
case 4: return "ZIP";
case 5: return "SOJ";
}
return number.ToString();
}
ИМХО легко читать и легко менять.
Я знаю, что этот ответ очень поздно, но мне было интересно, чтобы никто не выдвинул словарное решение еще. Я обнаружил, что когда имеешь дело с огромными операторами переключателей, которые ориентированы на данные, подобные этим, часто легче читать, свернуть случай переключения в словарь.
public static readonly IDictionary<int, string> Factors = new Dictionary<int, string> {
{ 1, "Other" },
{ 2, "SIP" },
{ 3, "DIP" },
{ 4, "ZIP" },
{ 5, "SOJ" }
};
public static string GetFormFactor2(int number)
{
string formFactor = string.Empty;
if (Factors.ContainsKey(number)) formFactor = Factors[number];
return formFactor;
}
Это дает вам показатель Maintainability, равный 74, - немного ниже, чем у массива, из-за связи классов со словарем, но я чувствую, что он более общий, потому что он работает для любого количества типов, которые вы обычно включаете. Как и решение с массивами, оно очень хорошо масштабируется и удаляет много повторяющегося кода.
Вообще говоря, использование подхода, управляемого данными, может помочь вашему коду быть более понятным, потому что он отделяет важные части (в данном случае, условие и результат) от лишнего (в данном случае, переключателя).
На ум приходят две вещи:
Используйте перечисление, чтобы объединить описание и значение
public enum FormFactor
{
Other = 1,
SIP = 2,
etc.
}
Используйте класс или структуру для представления каждого форм-фактора
public class FormFactor
{
public int Index { get; private set; }
public string Description { get; private set; }
public FormFactor(int index, string description)
{
// do validation and assign properties
}
}
Ясно, что метод Enum наиболее удобен в обслуживании, так как не содержит жестко запрограммированных строк, поэтому нет проблем с опечатками и проверкой синтаксиса во время компиляции. Только ограничения являются правилами именования.
Я не знаю, насколько это важно, но следующий получает 76:
private static string[] _formFactors = new[] { null, "Other","SIP","DIP", "ZIP", "SOJ"};
public static string GetFormFactor2(int number)
{
if (number < 1 || number > _formFactors.Length)
{
throw new ArgumentOutOfRangeException("number");
}
return _formFactors[number];
}