Являются ли жестко закодированные СТРОКИ когда-либо приемлемыми?

StackOverflow https://stackoverflow.com/questions/487485

Вопрос

Похожий на Являются ли литералы жесткого кодирования когда-либо приемлемыми?, но я конкретно имею в виду здесь "волшебные струны".

В крупном проекте у нас есть таблица параметров конфигурации, подобных этой:

Name         Value
----         -----
FOO_ENABLED  Y
BAR_ENABLED  N
...

(Их сотни).

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

if (config_options.value('FOO_ENABLED') == 'Y') ...

(Конечно, этот же параметр, возможно, потребуется проверить во многих местах системного кода.)

При добавлении новой опции я рассматривал возможность добавления функции для скрытия "волшебной строки" следующим образом:

if (config_options.foo_enabled()) ...

Однако коллеги подумали, что я перегнул палку, и возражали против этого, предпочитая жесткое кодирование, потому что:

  • Это то, что мы обычно делаем
  • Это облегчает понимание того, что происходит при отладке кода

Беда в том, что я понимаю их точку зрения!Реально, мы никогда не собираемся переименовывать параметры по какой-либо причине, поэтому единственное преимущество, которое я могу придумать для моей функции, заключается в том, что компилятор поймает любую опечатку, такую как fo_enabled(), но не 'FO_ENABLED' .

А ты как думаешь?Пропустил ли я какие-либо другие преимущества / недостатки?

Это было полезно?

Решение 

if (config_options.isTrue('FOO_ENABLED')) {...
}

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

if (config_options.isFooEnabled()) {...
}

Может показаться нормальным, пока у вас нет 100 вариантов конфигурации и 100 методов (так что здесь вы можете составить суждение о будущем росте приложения и потребностях, прежде чем принимать решение о вашей реализации).В противном случае лучше иметь класс статических строк для имен параметров.

if (config_options.isTrue(ConfigKeys.FOO_ENABLED)) {...
}

Другие советы

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

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

Если я использую строку в коде три раза, я почти наверняка сделаю ее константой.

Я понимаю, что вопрос старый, но он возник у меня на полях.

AFAIC, проблема здесь не была точно определена ни в вопросе, ни в ответах.Забудьте на мгновение о "харкодировании строк" или нет.

  1. В базе данных есть Справочная таблица, содержащая config_options.PK - это строка.

  2. Существует два типа PKS:

    • Значимые идентификаторы, которые видят и используют пользователи (и разработчики).Предполагается, что эти ПК стабильны, на них можно положиться.

    • Бессмысленный Id столбцы, которые пользователи никогда не должны видеть, о которых разработчики должны знать и обходить их кодом.На них нельзя полагаться.

  3. Это обычное дело - писать код, используя абсолютное значение значимого PK IF CustomerCode = "IBM" ... или IF CountryCode = "AUS" и т.д.

    • ссылка на абсолютное значение бессмысленного PK неприемлема (из-за автоматического увеличения;изменяемые пробелы;ценности заменяются оптом).
      .

  4. В вашей справочной таблице используются значимые PKS.Ссылки на эти литеральные строки в коде неизбежны.Скрытие значения усложнит обслуживание;код больше не является буквальным;ваши коллеги правы.Плюс есть дополнительная резервная функция, которая пережевывает циклы.Если в литерале есть опечатка, вы скоро обнаружите это во время тестирования разработчиков, задолго до UAT.

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

    • дело в том, что попытка скрыть литерал не имеет никакого значения.
      .

  5. Это не может быть истолковано как "жесткое кодирование", это нечто совсем другое.Я думаю, именно в этом заключается ваша проблема - идентифицировать эти конструкции как "жестко закодированные".Это просто буквальная ссылка на Осмысленный PK.

  6. Теперь, только с точки зрения любого сегмента кода, если вы используете одно и то же значение несколько раз, вы можете улучшить код, записав строку литерала в переменную, а затем используя переменную в остальной части блока кода.Конечно, это не функция.Но это вопрос эффективности и надлежащей практики.Даже это не меняет эффекта IF CountryCode = @cc_aus

По моему опыту, такого рода проблемы маскируют более глубокую проблему:неспособность выполнять фактическое ООП и следовать принципу DRY.

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

Подробности ниже.

Пример использования был:

if (config_options.value('FOO_ENABLED') == 'Y') ...

что вызывает очевидный вопрос: "Что происходит в многоточии?", особенно учитывая следующее утверждение:

(Конечно, этот же параметр, возможно, потребуется проверить во многих местах системного кода.)

Давайте предположим, что каждый из этих config_option значения действительно соответствуют концепции единой проблемной области (или стратегии внедрения).

Вместо того, чтобы делать это (неоднократно, в разных местах по всему коду):

  1. Возьмите строку (тег),
  2. Найдите соответствующую ему другую строку (значение),
  3. Проверьте это значение как логический эквивалент,
  4. Основываясь на этом тесте, решите, следует ли выполнять какое-либо действие.

Я предлагаю инкапсулировать концепцию "настраиваемого действия".

Давайте возьмем в качестве примера (очевидно, столь же гипотетического, как FOO_ENABLED ...;-) что ваш код должен работать либо в английских единицах, либо в метрических единицах.Если METRIC_ENABLED имеет значение "true", преобразует введенные пользователем данные из метрики в английский язык для внутренних вычислений и преобразует обратно перед отображением результатов.

Определите интерфейс:

public interface MetricConverter {
    double toInches(double length);
    double toCentimeters(double length);
    double toPounds(double weight);
    double toKilograms(double weight);
}

который идентифицирует в одном месте все поведение, связанное с концепцией METRIC_ENABLED.

Затем напишите конкретные реализации всех способов, которыми это поведение должно быть реализовано:

public class NullConv implements MetricConverter {
    double toInches(double length) {return length;}
    double toCentimeters(double length) {return length;}
    double toPounds(double weight)  {return weight;}
    double toKilograms(double weight)  {return weight;}
}

и

// lame implementation, just for illustration!!!!
public class MetricConv implements MetricConverter {
    public static final double LBS_PER_KG = 2.2D;
    public static final double CM_PER_IN = 2.54D
    double toInches(double length) {return length * CM_PER_IN;}
    double toCentimeters(double length) {return length / CM_PER_IN;}
    double toPounds(double weight)  {return weight * LBS_PER_KG;}
    double toKilograms(double weight)  {return weight / LBS_PER_KG;}
}

Во время запуска, вместо загрузки кучи config_options значения, инициализируйте набор настраиваемых действий, как в:

MetricConverter converter = (metricOption()) ? new MetricConv() : new NullConv();

(где выражение metricOption() выше приведена резервная копия для любой одноразовой проверки, которую вам нужно выполнить, включая просмотр значения METRIC_ENABLED ;-)

Затем, где бы ни было сказано в коде:

double length = getLengthFromGui();
if (config_options.value('METRIC_ENABLED') == 'Y') {
    length = length / 2.54D;
}
// do some computation to produce result
// ...
if (config_options.value('METRIC_ENABLED') == 'Y') {
    result = result * 2.54D;
}
displayResultingLengthOnGui(result);

перепишите это как:

double length = converter.toInches(getLengthFromGui());
// do some computation to produce result
// ...
displayResultingLengthOnGui(converter.toCentimeters(result));

Поскольку все детали реализации, связанные с этой концепцией, теперь упакованы в чистую упаковку, все будущее техническое обслуживание, связанное с METRIC_ENABLED может быть сделано в одном месте.Кроме того, компромисс во время выполнения - это выигрыш;"накладные расходы" на вызов метода тривиальны по сравнению с накладными расходами на извлечение строкового значения из Map и выполнение String#equals .

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

Вдобавок ко всему, вы можете получать типизированные функции, теперь кажется, что вы храните только логические значения, что, если вам нужно сохранить int, строку и т.д.Я бы предпочел использовать get_foo() с типом, чем get_string("FOO") или get_int("FOO").

Я действительно должен использовать константы и никаких жестко закодированных литералов.

Вы можете сказать, что они не будут изменены, но, возможно, вы никогда этого не узнаете.И лучше всего сделать это привычкой.Использовать символические константы.

Я думаю, что здесь есть две разные проблемы:

  • В текущем проекте соглашение об использовании жестко закодированных строк уже хорошо устоялось, поэтому все разработчики, работающие над проектом, знакомы с ним.Это может быть неоптимальным соглашением по всем перечисленным причинам, но каждый, кто знаком с кодом, может взглянуть на него и инстинктивно знает, что этот код должен делать.Изменение кода таким образом, чтобы в определенных частях он использовал "новую" функциональность, немного усложнит чтение кода (потому что людям придется подумать и запомнить, что делает новое соглашение) и, следовательно, немного усложнит обслуживание.Но я бы предположил, что переход всего проекта на новое соглашение потенциально был бы непомерно дорогостоящим, если вы не сможете быстро выполнить преобразование по сценарию.
  • На новое проект, символические константы - это способ IMO, по всем перечисленным причинам. Особенно потому что все, что заставляет компилятор перехватывать ошибки во время компиляции, которые в противном случае были бы перехвачены человеком во время выполнения, является очень полезным соглашением для установления.

Я тоже предпочитаю строго типизированный класс конфигурации, если он используется во всем коде.С правильно названными методами вы не теряете никакой удобочитаемости.Если вам нужно выполнить преобразования из строк в другой тип данных (decimal / float / int), вам не нужно повторять код, который выполняет преобразование в нескольких местах и может кэшировать результат, поэтому преобразование выполняется только один раз.У вас уже есть основа для этого, так что я не думаю, что потребуется много времени, чтобы привыкнуть к новое способ ведения дел.

Еще одна вещь, которую следует учитывать, - это намерение.Если вы работаете над проектом, требующим локализации, жестко закодированные строки могут быть неоднозначными.Рассмотрим следующее:

const string HELLO_WORLD = "Hello world!";
print(HELLO_WORLD);

Намерения программиста ясны.Использование константы подразумевает, что эту строку не нужно локализовывать.Теперь посмотрите на этот пример:

print("Hello world!");

Здесь мы не так уверены.Действительно ли программист не хотел, чтобы эта строка была локализована, или программист забыл о локализации, когда писал этот код?

Лицензировано под: CC-BY-SA с атрибуция
Не связан с StackOverflow
scroll top