use a static final Constant instand of duplicate Strings on `pushOntoCallStack(..)`

[stbischof]

would be greate if we could define a static final Constant in Parser and use this when call on pushOntoCallStack(..)

pushOntoCallStack("${currentProduction.name}", "${choice.inputSource?j_string}", ${choice.beginLine}, ${choice.beginColumn});

currently a lot of text is generated into the source

[revusky]

I'm pretty sure that your suggestion is based on a misreading. In Java, string literals are automatically "interned". Here you can convince yourself of this using this file:

     public class EqualityTest {
           static String s1 = "Hello";
           static public void main(String[] args) {
                    String s2 = "Hello";
                    System.out.println(s1==s2);
           }
     }

Compile the above and run it and see what it outputs. There are not two separate string objects because the string literals are interned, which means that the second declaration s2 = "Hello"; just reuses the first one. If instead you wrote: String s2 = new String("Hello"); then there would be a new string object and the program would output false.

[stbischof]

interesting. Thx .I did not new about this.

My Main issue are, that in my generated code is:

• a long String with absolut path. And a constant would be much more elegant.
• I like the phantasie that the generated code is free from issues from static code Analysis tools. I

[revusky]

Well, I have to be honest and just tell you that there is really no reason to be concerned about this. The reason to be concerned about string literals in human-written code is that, yes, they are a source of error. For example, if you had to write:

  "Deutschland"

a number of times in your code, maybe one time in 10, you write: "Deutchland" missing an s or some other spelling error. And the system does not catch this. Therefore, it is better to externalize the string, and write in one place:

    static final String DEUTSCHLAND="Deutschland";

An then you reuse that constant everywhere else. And now, if you misspell the name of the variable writing DEUTCHLAND, the compiler catches it because there is no variable with this name. OTOH, if you misspell it in the string literal, the compiler does not catch that. So your code becomes more fail-safe. This is the basic reason that the style checker says this is a "code smell".

But the warning really makes no sense in this template generated code since the string literal is programmatically generated anyway. For example, if you wrote:

  "${curentProduction.name}"

missing an 'r' in the variable name, the tool (the template engine in this case) would catch that and complain, since there is no variable called curentProduction. Ergo, there is absolutely no gain from externalizing this to some string constant. Presumably, you want the full path (used for error reporting mostly) to be a constant, as in:

        static final String INPUT_SOURCE  = "${choice.inputSource?j_string}";

and then elsewhere:

        pushOntoCallStack(CURRENT_PRODUCTION_NAME, INPUT_SOURCE, etc...);

But it doesn't gain you anything because the literal strings are programmatically generated anyway. They're not maintained by hand. Well, it's true that you can get rid of the warning, but it was a spurious warning anyway!

I suppose it's not necessarily such a bad idea to run generated code through a style checker. But really, one has to o this with the right mind, as it were. Frankly, you're taking these various warnings that it emits far too seriously.

[vsajip]

Shall I close this issue? I don't think there's anything to be done here.

[vsajip]

Closed, as no contrary feedback received.