Почему этот индекс ремонтопригодности увеличивается?

Я был бы признателен, если бы кто-нибудь объяснил мне разницу между следующими двумя фрагментами кода с точки зрения правил Visual Studio Code Metrics. Почему индекс ремонтопригодности немного увеличивается, если я не инкапсулирую все в using ( )?

Образец 1 (оценка МИ 71)

public static String Sha1(String plainText)
{
    using (SHA1Managed sha1 = new SHA1Managed())
    {
        Byte[] text = Encoding.Unicode.GetBytes(plainText);
        Byte[] hashBytes = sha1.ComputeHash(text);
        return Convert.ToBase64String(hashBytes);    
    }
}

Образец 2 (оценка МИ 73)

public static String Sha1(String plainText)
{
    Byte[] text, hashBytes;
    using (SHA1Managed sha1 = new SHA1Managed())
    {
        text = Encoding.Unicode.GetBytes(plainText);
        hashBytes = sha1.ComputeHash(text);
    }
    return Convert.ToBase64String(hashBytes);   
}

Я понимаю, что метрики бессмысленны вне более широкого контекста и понимания, и программисты должны проявлять осторожность. Хотя я мог повысить оценку до 76 с помощью return Convert.ToBase64String(sha1.ComputeHash(Encoding.Unicode.GetBytes(plainText))), я не должен этого делать. Очевидно, я бы просто играл с числами, и на тот момент это действительно не было бы более читабельным или удобным для обслуживания. Мне любопытно, однако, какая логика может стоять за увеличением в этом случае. Это явно не количество строк.


person Timothy    schedule 01.05.2010    source источник
comment
Обсуждение этого вопроса и различных ответов на него предполагает, что индекс ремонтопригодности не очень интуитивен — см. также мой опубликовать об индексе ремонтопригодности, обсуждая различные проблемы с этой метрикой.   -  person avandeursen    schedule 13.09.2014


Ответы (4)


Размещение всех ваших переменных вверху, чтобы вы знали, что находится в функции, является более «поддерживаемым», по крайней мере, так думает тот, кто определяет правила для метрик кода.

Так ли это на самом деле? Полностью зависит от команды, работающей над кодом. Кажется, вы уже знаете это по тону вопроса, но воспринимайте почти все метрики кода с долей скептицизма, они кто-то считает их лучшими, что может быть неверно для команд за пределами Microsoft... делайте то, что лучше для вашей команды, а не то, что подсказывает вам калькулятор.

Я бы не стал вносить изменения, которые наносят ущерб вашей производительности и производительности кодирования вашей команды (если только это не связано с реальной производительностью или улучшенной обработкой ошибок и т. д.), которые, по вашему мнению, менее удобочитаемы для получения нескольких баллов на доске показателей.

При всем при этом, если это дает вам очень низкую ремонтопригодность, вероятно, есть что-то, на что стоит обратить внимание или разбить на более мелкие фрагменты, так как очень низкий балл, вероятно, не так уж приемлем практически для любого команда.

person Nick Craver    schedule 01.05.2010
comment
Я считаю, что это правильно - с точки зрения объяснения цифр, - но я думаю, что расчет ремонтопригодности в этом отношении устарел. Когда-то имело смысл объявлять все ваши переменные вверху — когда-то (некоторые) языки требовали этого! - но это время давно прошло. Сегодня минимизация срока службы идентификатора в большей степени способствует удобству сопровождения. - person Carl Manaster; 01.05.2010
comment
Это неверно: метрики не заботятся о расположении переменных (хотя я думаю, что к настоящему времени уже принято считать, что близость к использованию является плюсом). Как указывает Дэн Брайант, вы просто видите эффект объявления двух переменных в одной строке (это означает, что Byte[] появляется только один раз во втором методе), что делает метод короче с точки зрения объема Холстеда. - person StriplingWarrior; 21.03.2014

Это старый вопрос, но я просто решил добавить, что MI частично основан на объеме Холстеда, который основан на подсчете «операторов» и «операндов». Если объявление переменной по типу является «оператором», это будет означать, что в примере 2 меньше операторов, что изменит оценку. В целом, поскольку МИ является статистическим измерением, его полезность при работе с небольшими выборками (например, с одним коротким методом) ограничена.

person Dan Bryant    schedule 25.07.2012
comment
Интересный момент, интересно, уменьшится ли счет, если разделить текст Byte[], hashBytes на две строки - person Mark Sowul; 25.11.2013
comment
@MarkSowul: это не повлияет на Холстеда, но повлияет на строки кода, которые также являются компонентом индекса ремонтопригодности. - person avandeursen; 13.09.2014

Из-за увеличенного расстояния между объявлением ваших переменных и тем, где они используются.

Правило состоит в том, чтобы максимально уменьшить диапазон переменных, диапазон — это расстояние между объявлением переменной и тем, где она используется. По мере увеличения этого расстояния увеличивается риск того, что позже будет введен код, влияющий на переменную, а программист не осознает влияние более глубокого кода.

Вот ссылка на хорошую книгу, которая охватывает эту и многие другие темы качества кода. https://rads.stackoverflow.com/amzn/click/com/0735619670

person Chris Taylor    schedule 01.05.2010
comment
Это может показаться противоречащим вопросу, поскольку расстояние между объявлением и использованием больше в более высоком балле. - person Nick Craver; 01.05.2010
comment
@Ник правда, но странно. Я нахожу версию с более низким баллом намного легче для чтения, и объяснение @Chris кажется мне вполне разумным. - person Damian Powell; 01.05.2010
comment
@Damian - Объяснение отличается от результата, разумно, я согласен, но не объясняет вопрос. Я также предпочитаю тот, у которого более низкий балл, но, согласно этому ответу, он должен иметь более высокий балл, но это не так. - person Nick Craver; 01.05.2010
comment
@Nick Ник, я задавался вопросом, неправильно ли ОП получил значения. Я тестировал это в VS2010, и это не так. Хотя у меня получаются разные значения: 70 и 69. Странно, но подчёркивает ваш тезис, что его надо воспринимать с долей скептицизма. - person Damian Powell; 01.05.2010
comment
Я думал об этом, и хотя я предполагаю, что только кто-то, тесно связанный с механизмом анализа, может дать реальное объяснение этому, мне интересно, измеряет ли механизм анализа длину строки и говорит ли, что сложность объявления и инициализации переменных путем вызова функций влияет на метрику. У меня сейчас нет доступа к VS, что может быть интересно попробовать, так это взять первый код и переместить объявление переменной в область использования предложения, но все еще инициализировать на отдельных строках, что это дает? - person Chris Taylor; 01.05.2010
comment
Те же цифры, @Chris. Меньшее значение, когда переменные определены и используются внутри блока using(), чем когда они определены вне блока. - person Timothy; 02.05.2010
comment
@ Тимоти, спасибо за обновление. Должен признаться, я думаю, что это всего лишь особенность анализа, но опять же я сомневаюсь, что кто-либо за пределами команды, разрабатывающей технологию анализа, может дать конкретный ответ. Разница, конечно, невелика, так что вас это не должно волновать, но с интеллектуальной точки зрения интересно! - person Chris Taylor; 02.05.2010
comment
Диапазон текста и хэш-байтов больше во втором, но диапазон переменной в блоке использования становится короче. Я бы предположил, что уменьшение количества операторов в блоке с использованием скорости выше, чем уменьшение диапазона более простых переменных. - person Caleb Mauer; 19.05.2016

Сам я бы предпочел увидеть return Convert.ToBase64String(sha1.ComputeHash(Encoding.Unicode.GetBytes(plainText))); это следует, а не не следует. Преимущество этой формы заключается в кратком выражении фактического потока данных; если вы добавите кучу временных переменных и присваиваний, мне теперь придется читать имена переменных и сопоставлять их вхождения, чтобы увидеть, что происходит на самом деле.

person Kevin Reid    schedule 02.05.2010
comment
Я не согласен. В одной строке вы должны сначала визуально сканировать, чтобы найти самое внутреннее выражение (plainText), а затем назад, к GetBytes. Хорошо, у нас есть байты текста. Затем sha1.ComputeHash (хорошо, теперь мы берем SHA1), а затем, наконец, основание 64. Если вы разобьёте это на пару строк, я думаю, будет намного проще пройти построчно и понять, что происходит. на. Если кто-то выбирает интеллектуальные имена переменных, вам не нужно сопоставлять их вхождения, это просто имеет смысл. - person Jonathon Reinhart; 16.08.2012
comment
На самом деле однострочник выражает полную противоположность потоку данных, потому что вызовы методов в основном представляют собой префиксную нотацию. Фактический поток выглядит следующим образом: обычный текст -> GetBytes -> ComputeHash -> ToBase64String. Если разложить построчно, это видно. Однострочник, который вы должны читать справа налево. А отлаживать гораздо сложнее, потому что вы не видите промежуточных шагов. Представьте, что существует исключение нулевой ссылки. Строка за строкой покажет вам сразу. Как насчет однострочного? - person Mark Sowul; 15.10.2013
comment
@MarkSowul Я согласен на 100%. Я всегда предпочитаю удобочитаемость, чем один лайнер с несколькими цепочками функций. Если один из членов моей команды сделает это, я скажу ему/ей провести рефакторинг. - person Eriawan Kusumawardhono; 28.10.2013