Постоянное насилие? - PullRequest
Новая
       35

Постоянное насилие?

19 голосов
/ 07 декабря 2009

Я столкнулся с кучей кода в нескольких проектах на C #, которые имеют следующие константы: 

    const int ZERO_RECORDS = 0;
    const int FIRST_ROW = 0;
    const int DEFAULT_INDEX = 0;
    const int STRINGS_ARE_EQUAL = 0;

Кто-нибудь когда-нибудь видел что-нибудь подобное? Есть ли способ рационализировать использование констант для представления языковых конструкций? IE: первый индекс C # в массиве находится в позиции 0. Я бы подумал, что если разработчик должен зависеть от константы, чтобы сказать им, что язык основан на 0, то есть большая проблема под рукой.

Чаще всего эти константы используются при обработке таблиц данных или внутри циклов for.

Я неуместен, думая, что это запах кода? Я чувствую, что это не намного лучше, чем: 

const int ZERO = 0;
const string A = "A";

Ответы [ 17 ]

12 голосов
/ 07 декабря 2009

Я неуместен, думая, что это запах кода? Я чувствую, что это не намного лучше, чем:

Сравните следующее: 

if(str1.CompareTo(str2) == STRINGS_ARE_EQUAL) ...

с 

if(str1.CompareTo(str2) == ZERO) ...
if(str1.CompareTo(str2) == 0) ...

Какой из них имеет более непосредственный смысл?

10 голосов
/ 07 декабря 2009

Злоупотребление, ИМХО. «Ноль» - это просто одна из основ.

Хотя STRINGS_ARE_EQUAL может быть простым, почему бы не ".Equals"?

Принято ли ограниченное использование магических чисел?

5 голосов
/ 07 декабря 2009

Некоторые люди считают любое необработанное число в программе «магическим числом». Я видел стандарты кодирования, которые в основном говорили, что вы не можете просто записать целое число в программу, это должно быть const int.

5 голосов
/ 07 декабря 2009

Это определенно запах кода.

Возможно, целью было «добавить читабельность» к коду, однако, по моему мнению, подобные вещи на самом деле снижают читабельность кода.

3 голосов
/ 07 декабря 2009

Это определенно плохое кодирование.

Я говорю, что константы следует использовать только там, где это необходимо, когда вещи могут измениться когда-нибудь позже. Например, у меня есть много опций «конфигурации», таких как 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
3 голосов
/ 07 декабря 2009

Я неуместен, думая, что это запах кода? Я чувствую, что это не намного лучше, чем:

const int ZERO = 0;

const int A = 'A';

Вероятно, немного обоняние, но определенно лучше, чем ZERO = 0 и A = 'A'. В первом случае они определяют логические константы, то есть некоторую абстрактную идею (равенство строк) с конкретной реализацией значения.

В вашем примере вы определяете литеральные константы - переменные представляют собой сами значения. Если это так, я думаю, что перечисление предпочтительнее, поскольку они редко являются единичными значениями.

2 голосов
/ 07 декабря 2009

Вы должны взглянуть на некоторые вещи на thedailywtf

One2Pt20462262185th

и

Корпоративный SQL

2 голосов
/ 08 декабря 2009

Я думаю, что иногда люди слепо следуют «стандартам кодирования» , которые говорят: «Не используйте жестко закодированные значения, определите их как константы, чтобы было проще управлять кодом, когда его нужно обновить», - что достаточно справедливо для таких вещей, как: 

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 value или что-то в этом роде (я только что сделал это), тогда вы бы не использовали 0, а соответствующую переменную.

Так что это зависит от того, когда вам конкретно нужен доступ к первому элементу в массиве arr[0] лучше, чем arr[FIRST_ELEMENT], потому что я все равно пойду и проверю то, что вы определили как FIRST_ELEMENT, потому что я не буду вам доверять, и это может быть чем-то отличным от 0 - например, ваш 0 элемент dud и настоящий первый элемент хранится в 1 - кто знает.

1 голос
/ 08 декабря 2009

Обычно существует четыре причины использования константы:

  1. В качестве замены значения, которое может разумно измениться в будущем (например, IdColumnNumber = 1).
  2. В качестве метки для значения, которое может быть непростым для понимания или значимым само по себе (например, FirstAsciiLetter = 65),
  3. Как более короткий и менее подверженный ошибкам способ ввода длинного или трудного для ввода значения (например, LongSongTitle = "Supercalifragilisticexpialidocious")
  4. в качестве вспомогательного средства для запоминания значения, которое трудно запомнить (например, 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".

1 голос
/ 08 декабря 2009

Этот код находится в вашем офисе или загружен?

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

В C # в идеале вы хотите создать класс, который содержит константы, которые используются глобально каждым другим классом. Например, 

class MathConstants
{
 public const int ZERO=0;
}

Тогда в более поздних классах что-то вроде: 

....
if(something==MathConstants.ZERO)
...

По крайней мере, так я это вижу. Таким образом, каждый может понять, что это за константы, даже не читая ничего другого. Это уменьшит путаницу.

Страница:
...