Uploaded image for project: 'Groovy'
  1. Groovy
  2. GROOVY-8547

Factory pattern side-stepped by CompilerConfiguration/ParserVersion

    XMLWordPrintableJSON

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.0-beta-3
    • Component/s: parser
    • Labels:
      None

      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.

          public ParserPluginFactory getPluginFactory() {
              if (pluginFactory == null) {
                  pluginFactory = new ParserPluginFactory() {
                      @Override
                      public ParserPlugin createParserPlugin() {
                          return Boolean.getBoolean(GROOVY_ANTLR4_OPT) ? new Antlr4ParserPlugin() : new AntlrParserPlugin();
                      }
                  }              
              }
              return pluginFactory;
          }
      

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                paulk Paul King
                Reporter:
                emilles Eric Milles
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 40m
                  40m