[
https://issues.apache.org/jira/browse/GROOVY-8547?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Eric Milles updated GROOVY-8547:
--------------------------------
Description:
It seems to me the selection of the Antlr2 or Antlr4 parser could be
implemented within a default implementation of {{ParserPluginFactory}} instead
of introducing {{ParserVersion}} with separate get/set on
{{CompilerConfiguration}}. I don't see the need for static factory methods
{{antlr2()}} and {{antlr4()}} on {{ParserPluginFactory}}, their use from
{{CompilerConfiguration}} and the strange class loading in
{{ParserPluginFactory}}.
The factory pattern appears to me to be side-stepped in favor of a version
constant. Is there still meaning in offering a config option of setting a
{{ParserPluginFactory}}? Has this been used ever in the past?
Couldn't this suffice for a default in {{CompilerConfiguration}}? Then
{{ParserVersion}} could be dropped.
{code:java}
public ParserPluginFactory getPluginFactory() {
if (pluginFactory == null) {
pluginFactory = new ParserPluginFactory() {
@Override
public ParserPlugin createParserPlugin() {
return Boolean.getBoolean(GROOVY_ANTLR4_OPT) ? new
Antlr4PluginFactory() : new AntlrParserPluginFactory();
}
}
}
return pluginFactory;
}
{code}
was:
It seems to me the selection of the Antlr2 or Antlr4 parser could be
implemented within a default implementation of {{ParserPluginFactory}} instead
of introducing {{ParserVersion}} with separate get/set on
{{CompilerConfiguration}}. I don't see the need for static factory methods
{{antlr2()}} and {{antlr4()}} on {{ParserPluginFactory}}, their use from
{{CompilerConfiguration}} and the strange class loading in
{{ParserPluginFactory}}.
The factory pattern appears to me to be side-stepped in favor of a version
constant. Is there still meaning is offering a config option of setting a
{{ParserPluginFactory}}? Has this been used ever in the past?
Couldn't this suffice for a default in {{CompilerConfiguration}}? Then
{{ParserVersion}} could be dropped.
{code:java}
public ParserPluginFactory getPluginFactory() {
if (pluginFactory == null) {
pluginFactory = new ParserPluginFactory() {
@Override
public ParserPlugin createParserPlugin() {
return Boolean.getBoolean(GROOVY_ANTLR4_OPT) ? new
Antlr4PluginFactory() : new AntlrParserPluginFactory();
}
}
}
return pluginFactory;
}
{code}
> Factory pattern side-stepped by CompilerConfiguration/ParserVersion
> -------------------------------------------------------------------
>
> Key: GROOVY-8547
> URL: https://issues.apache.org/jira/browse/GROOVY-8547
> Project: Groovy
> Issue Type: Improvement
> Components: parser
> Reporter: Eric Milles
> Priority: Minor
>
> It seems to me the selection of the Antlr2 or Antlr4 parser could be
> implemented within a default implementation of {{ParserPluginFactory}}
> instead of introducing {{ParserVersion}} with separate get/set on
> {{CompilerConfiguration}}. I don't see the need for static factory methods
> {{antlr2()}} and {{antlr4()}} on {{ParserPluginFactory}}, their use from
> {{CompilerConfiguration}} and the strange class loading in
> {{ParserPluginFactory}}.
> The factory pattern appears to me to be side-stepped in favor of a version
> constant. Is there still meaning in offering a config option of setting a
> {{ParserPluginFactory}}? Has this been used ever in the past?
> Couldn't this suffice for a default in {{CompilerConfiguration}}? Then
> {{ParserVersion}} could be dropped.
> {code:java}
> public ParserPluginFactory getPluginFactory() {
> if (pluginFactory == null) {
> pluginFactory = new ParserPluginFactory() {
> @Override
> public ParserPlugin createParserPlugin() {
> return Boolean.getBoolean(GROOVY_ANTLR4_OPT) ? new
> Antlr4PluginFactory() : new AntlrParserPluginFactory();
> }
> }
> }
> return pluginFactory;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
