Ответ 1
Злоупотребление, ИМХО. "Ноль" - это лишь одна из основ.
Хотя STRINGS_ARE_EQUAL может быть легко, почему бы не ".Equals"?
Я столкнулся с кучей кода в нескольких проектах С#, которые имеют следующие константы:
const int ZERO_RECORDS = 0;
const int FIRST_ROW = 0;
const int DEFAULT_INDEX = 0;
const int STRINGS_ARE_EQUAL = 0;
Кто-нибудь видел что-нибудь подобное? Есть ли способ рационализировать использование констант для представления языковых конструкций? IE: первый индекс С# в массиве находится в позиции 0. Я бы подумал, что если разработчик должен зависеть от константы, чтобы сказать им, что язык основан на 0, существует большая проблема.
Наиболее частое использование этих констант заключается в обработке таблиц данных или внутри циклов "for".
Неужели я считаю, что это запах кода? Я чувствую, что это не намного лучше, чем:
const int ZERO = 0;
const string A = "A";
Злоупотребление, ИМХО. "Ноль" - это лишь одна из основ.
Хотя STRINGS_ARE_EQUAL может быть легко, почему бы не ".Equals"?
Неужели я считаю, что это запах кода? Я чувствую, что это не намного лучше, чем:
Сравните следующее:
if(str1.CompareTo(str2) == STRINGS_ARE_EQUAL) ...
с
if(str1.CompareTo(str2) == ZERO) ...
if(str1.CompareTo(str2) == 0) ...
Какой из них имеет более непосредственный смысл?
Это определенно запах кода.
Возможно, намерение заключалось в том, чтобы "добавить читаемость" к коду, однако подобные вещи фактически уменьшают читаемость кода, на мой взгляд.
Некоторые люди считают, что любое количество необработанных чисел в программе является "магическим числом". Я видел стандарты кодирования, которые в основном говорили, что вы не можете просто написать целое число в программу, оно должно быть const int.
Неужели я считаю, что это запах кода? Я чувствую, что это не намного лучше, чем:
const int ZERO = 0;
const int A = 'A';
Вероятно, немного запаха, но определенно лучше, чем ZERO = 0 и A = 'A'. В первом случае они определяют логические константы, т.е. Некоторую абстрактную идею (равенство строк) с реализацией конкретного значения.
В вашем примере вы определяете литеральные константы - переменные представляют сами значения. Если это так, я думаю, что перечисление предпочтительнее, поскольку они редко являются сингулярными значениями.
Это определенно плохое кодирование.
Я говорю, что константы должны использоваться только там, где это необходимо, когда что-то может измениться через некоторое время. Например, у меня есть много опций "конфигурации", например SESSION_TIMEOUT
, где он должен оставаться неизменным, но, возможно, он может быть изменен позже по дороге. Я не думаю, что ZERO
может быть изменен по дороге.
Кроме того, для магических чисел не следует включать ноль.
Я немного странно, я думаю об этом убеждении, хотя, потому что я бы сказал, что что-то вроде этого уходит далеко.
//input is FIELD_xxx where xxx is a number
input.SubString(LENGTH_OF_FIELD_NAME); //cut out the FIELD_ to give us the number
Вы должны взглянуть на некоторые вещи в thedailywtf
и
Я думаю, что иногда люди слепо следуют "Стандартам кодирования" , которые говорят: "Не используйте жестко заданные значения, определяйте их как константы, чтобы упростить управление кодом, когда он нуждается в обновлении" - что достаточно справедливо для таких вещей, как:
const in MAX_NUMBER_OF_ELEMENTS_I_WILL_ALLOW = 100
Но не имеет смысла:
if(str1.CompareTo(str2) == STRINGS_ARE_EQUAL)
Поскольку каждый раз, когда я вижу этот код, мне нужно искать то, что STRINGS_ARE_EQUAL
определяется как, а затем проверить с помощью документов, если это правильно.
Вместо этого, если я вижу:
if(str1.CompareTo(str2) == 0)
Я пропускаю шаг 1 (поиск, который STRINGS_ARE...
определяется как), и может проверять спецификации для значения 0
.
Вы хотели бы, чтобы вы заменили это на Equals()
и использовали CompareTo()
в тех случаях, когда вас больше интересует только один случай, например:
switch (bla.CompareTo(bla1))
{
case IS_EQUAL:
case IS_SMALLER:
case IS_BIGGER:
default:
}
с помощью if/else
операторов, если это необходимо (не знаю, что возвращает CompareTo()
...)
Я бы по-прежнему проверял, правильно ли вы определили значения в соответствии со спецификациями.
Это, конечно, отличается, если спецификации определяют что-то вроде значения ComparisonClass::StringsAreEqual
или что-то вроде этого (я только что сделал это), тогда вы не будете использовать 0, а соответствующую переменную.
Итак, это зависит от того, когда вам нужно получить доступ к первому элементу в массиве arr[0]
, лучше, чем arr[FIRST_ELEMENT]
, потому что я по-прежнему буду проверять, что вы определили как FIRST_ELEMENT
, потому что я не буду доверять вам, и это может что-то отличное от 0
- например, ваш элемент 0
является dud, а реальный первый элемент хранится в 1
- кто знает.
Я поел бы запах кода. Если эти типы констант необходимы, поместите их в перечисление:
enum StringEquality
{
Equal,
NotEqual
}
(Однако я подозреваю, что STRINGS_ARE_EQUAL
- это то, что возвращается string.Compare
, поэтому взломать его для возврата перечисления может быть еще более подробным.)
Изменить: Также SHOUTING_CASE
не является особенно . Соглашение об именах в стиле NET.
Я не знаю, назову ли я их запахами, но они кажутся излишними. Хотя DEFAULT_INDEX действительно может быть полезным.
Дело в том, что избегать магических чисел, а нули на самом деле не волшебны.
Этот код что-то в вашем офисе или что-то вы загрузили?
Если это в офисе, я думаю, что это проблема с управлением, если люди случайно помещают константы. В глобальном масштабе не должно быть никаких констант, если у каждого не будет четкой идеи или согласия относительно того, для чего используются эти константы.
В С# в идеале вы хотите создать класс, который содержит константы, которые используются глобально всеми другими классами. Например,
class MathConstants
{
public const int ZERO=0;
}
Затем в более поздних классах что-то вроде:
....
if(something==MathConstants.ZERO)
...
По крайней мере, как я это вижу. Таким образом, каждый может понять, что такое константы, даже не прочитав ничего. Это уменьшит путаницу.
Обычно для использования константы я могу предположить четыре причины:
IdColumnNumber = 1
).FirstAsciiLetter = 65
),LongSongTitle = "Supercalifragilisticexpialidocious"
)PI = 3.14159265
)Для ваших конкретных примеров, вот как я буду судить каждый пример:
const int ZERO_RECORDS = 0;
// almost definitely a code smell
const int FIRST_ROW = 0;
// first row could be 1 or 0, so this potentially fits reason #2,
// however, doesn't make much sense for standard .NET collections
// because they are always zero-based
const int DEFAULT_INDEX = 0;
// this fits reason #2, possibly #1
const int STRINGS_ARE_EQUAL = 0;
// this very nicely fits reason #2, possibly #4
// (at least for anyone not intimately familiar with string.CompareTo())
Итак, я бы сказал, что нет, это не хуже Zero = 0
или A = "A"
.
Если нуль указывает что-то отличное от нуля (в данном случае STRINGS_ARE_EQUAL), то это IS Magical. Создание константы для него является приемлемым и делает код более удобочитаемым.
Создание константы с именем ZERO бессмысленно и трата энергии пальца!
Понюхает немного, но я мог видеть случаи, когда это имело бы смысл, особенно если у вас все время программисты переключаются с языка на язык.
Например, MATLAB одноиндексирован, поэтому я мог представить, что кто-то устает от ошибок при каждом переключении языков и определении DEFAULT_INDEX в программах С++ и MATLAB, чтобы абстрагировать разницу. Не обязательно элегантный, но если это то, что нужно...
Правильно, вы должны подвергнуть сомнению этот запах молодого кодового воина. Однако эти именованные константы основаны на практике кодирования, намного старше, чем рассвет Visual Studio. Вероятно, они избыточны, но вы можете сделать хуже, чем понять происхождение конвенции. Думайте о компьютерах NASA, назад, когда...
Вы можете увидеть что-то подобное в кросс-платформенной ситуации, когда вы будете использовать файл с набором констант, подходящих для платформы. Но, вероятно, не с этими фактическими примерами. Это похоже на то, что кодер COBOL пытался сделать свой С# больше похожим на английский язык (без нарушений, предназначенных для кодеров COBOL).
Все в порядке использовать константы для представления абстрактных значений, но совсем другое - представлять конструкции на вашем родном языке.
const int FIRST_ROW = 0
не имеет смысла.
const int MINIMUM_WIDGET_COUNT = 0
имеет больше смысла.
Предпосылка того, что вы должны следовать стандарту кодирования, имеет смысл. (То есть стандарты кодирования предположительно правильны внутри организации.) Славянское следование за ним, когда презумпция не выполняется, не имеет смысла.
Поэтому я согласен с более ранними плакатами о том, что некоторые вонючие константы, вероятно, были результатом следующего стандарта кодирования ( "без магических чисел" ) без буквы "письмо". Это проблема здесь.