English 中文(简体)
硬编码的字符串是否可以接受?
原标题:
  • 时间:2009-01-28 12:45:05
  •  标签:

类似于编码字面量是否可接受?,但我在这里特别考虑的是“神奇的字符串”。

在一个大型项目中,我们有一个如下的配置选项表:

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

数百个。

通常的做法是调用通用函数来测试像这样的选项:

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

当然,在系统代码的许多地方可能需要检查同样的选项。

当增加一个新选项时,我正在考虑添加一个隐藏“魔术字符串”的功能,如下所示:

if (config_options.foo_enabled()) ...

然而,同事们认为我有些过火,并反对这样做,更喜欢硬编码,因为:

  • That s what we normally do
  • It makes it easier to see what s going on when debugging the code

问题是,我能理解他们的观点!实现上,我们永远不会为任何原因重命名选项,所以我能想到的仅有的好处是编译器会捕捉到任何拼写错误,如 fo_enabled() 而不是 FO_ENABLED。

你怎么想?我有没有忽略其他的优点/缺点?

最佳回答
if (config_options.isTrue( FOO_ENABLED )) {...
}

将硬编码的Y检查限制在一个地方,即使这意味着为您的Map编写包装类。

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

可能看起来还好,直到你有100个配置选项和100个方法(因此在决定实现之前,您可以对未来应用程序的增长和需求进行评估)。否则最好有一个静态字符串类来存储参数名称。

if (config_options.isTrue(ConfigKeys.FOO_ENABLED)) {...
}
问题回答

如果我在代码中只使用一次字符串,通常不会担心在某个地方将其常量化。

如果我在代码中两次使用了一个字符串,我会考虑将其设置为常量。

如果我在代码中使用了一个字符串三次,我几乎肯定会把它变成一个常量。

我意识到这个问题已经很久了,但它出现在我的页边上。

以我所知,在这里问题并没有被准确地识别,无论是在问题中还是答案中。暂时先忘记“硬编码字符串”或不硬编码字符串的问题。

  1. 该数据库拥有一个引用表,包含 config_options。主键为字符串类型。

  2. 有两种类型的PK:

    • 有意义的标识符,用户(和开发人员)可以看到和使用。这些PK应该是稳定的,可以依靠。

    • 无意义的Id列,用户永远不应该看到,但开发人员必须意识到并编写代码以排除这些列。这些列不能被信任。

  3. 使用有意义的主键的绝对值编写代码是普通的、正常的,例如IF CustomerCode = "IBM" ...IF CountryCode = "AUS"等。

    • referencing the absolute value of a meaningless PK is not acceptable (due to auto-increment; gaps being changed; values being replaced wholesale).
      .
  4. 你的参考表使用有意义的主键。在代码中引用这些字面字符串是不可避免的。隐藏值将使维护更加困难;代码不再是字面的;你的同事是正确的。此外还有额外的冗余功能会耗费循环。如果字面上有拼写错误,你很快就会在开发测试中发现这一点,长时间在UAT之前。

    • 数百个字面量的数百个功能是荒谬的。如果您要实现一个函数,则应规范化您的代码并提供一个单一的函数,可以用于任何数百个字面量之一。在这种情况下,我们又回到了裸露的文本,可以放弃该函数。

    • the point is, the attempt to hide the literal has no value.
      .

  5. 它不能被解释为“硬编码”,那是完全不同的事情。我认为你的问题在于,将这些结构体解释为“硬编码”。它仅仅是字面意义上引用了一个有意义的主键。

  6. 现在从任何代码段的角度来看,如果您多次使用相同的值,可以通过将字面字符串捕获到变量中,然后在代码块的其余部分中使用该变量来改善代码。当然不是一个函数。但这是一项效率和良好实践问题。即使这样也不会改变效果IF CountryCode = @cc_aus

我真的应该使用常量而不是硬编码的字面量。

你可以说它们不会改变,但你可能永远不知道。最好养成习惯。使用符号常数。

In my experience, this kind of issue is masking a deeper problem: failure to do actual OOP and to follow the DRY principle.

In a nutshell, capture the decision at startup time by an appropriate definition for each action inside the if statements, and then throw away both the config_options and the run-time tests.

Details below.

The sample usage was:

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

which raises the obvious question, "What s going on in the ellipsis?", especially given the following statement:

(Of course, this same option may need to be checked in many places in the system code.)

Let s assume that each of these config_option values really does correspond to a single problem domain (or implementation strategy) concept.

Instead of doing this (repeatedly, in various places throughout the code):

  1. Take a string (tag),
  2. Find its corresponding other string (value),
  3. Test that value as a boolean-equivalent,
  4. Based on that test, decide whether to perform some action.

I suggest encapsulating the concept of a "configurable action".

Let s take as an example (obviously just as hypthetical as FOO_ENABLED ... ;-) that your code has to work in either English units or metric units. If METRIC_ENABLED is "true", convert user-entered data from metric to English for internal computation, and convert back prior to displaying results.

Define an interface:

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

which identifies in one place all the behavior associated with the concept of METRIC_ENABLED.

Then write concrete implementations of all the ways those behaviors are to be carried out:

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;}
}

and

// 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;}
}

At startup time, instead of loading a bunch of config_options values, initialize a set of configurable actions, as in:

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

(where the expression metricOption() above is a stand-in for whatever one-time-only check you need to make, including looking at the value of METRIC_ENABLED ;-)

Then, wherever the code would have said:

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);

rewrite it as:

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

Because all of the implementation details related to that one concept are now packaged cleanly, all future maintenance related to METRIC_ENABLED can be done in one place. In addition, the run-time trade-off is a win; the "overhead" of invoking a method is trivial compared with the overhead of fetching a String value from a Map and performing String#equals.

I believe that the two reasons you have mentioned, Possible misspelling in string, that cannot be detected until run time and the possibility (although slim) of a name change would justify your idea.

On top of that you can get typed functions, now it seems you only store booleans, what if you need to store an int, a string etc. I would rather use get_foo() with a type, than get_string("FOO") or get_int("FOO").

I think there are two different issues here:

  • In the current project, the convention of using hard-coded strings is already well established, so all the developers working on the project are familiar with it. It might be a sub-optimal convention for all the reasons that have been listed, but everybody familiar with the code can look at it and instinctively knows what the code is supposed to do. Changing the code so that in certain parts, it uses the "new" functionality will make the code slightly harder to read (because people will have to think and remember what the new convention does) and thus a little harder to maintain. But I would guess that changing over the whole project to the new convention would potentially be prohibitively expensive unless you can quickly script the conversion.
  • On a new project, symbolic constants are the way IMO, for all the reasons listed. Especially because anything that makes the compiler catch errors at compile time that would otherwise be caught by a human at run time is a very useful convention to establish.

Another thing to consider is intent. If you are on a project that requires localization hard coded strings can be ambiguous. Consider the following:

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

The programmer s intent is clear. Using a constant implies that this string does not need to be localized. Now look at this example:

print("Hello world!");

Here we aren t so sure. Did the programmer really not want this string to be localized or did the programmer forget about localization while he was writing this code?

I too prefer a strongly-typed configuration class if it is used through-out the code. With properly named methods you don t lose any readability. If you need to do conversions from strings to another data type (decimal/float/int), you don t need to repeat the code that does the conversion in multiple places and can cache the result so the conversion only takes place once. You ve already got the basis of this in place already so I don t think it would take much to get used to the new way of doing things.





相关问题
热门标签