Re: [patch #5603] Separate command's parsing from their execution

[Ben Pfaff]

John Darrington <address@hidden> writes:

> This patch is a necessary step to allow commands to be run from the GUI, and
> also if we want to do something like the Python Plug-in.
>
> It introduces a new programming interface, which deals with  the parsing of
> commands, as a seperate process from their execution.
> Porting all the commands to the new interface, will be a long, mundane and
> monotonous business, so this patch allows us to have a transition period,
> where both the new and the old interfaces will be supported.

I think what you're getting at is that we want two interfaces to
most commands: one that takes a lexer and runs the command;
another that takes a specification and runs the command.

Here is the way that I had envisioned doing this: keep the
existing command.c structure.  Then, for each command that wants
to be executable from a specification, divide it into two pieces:
a parser and an executor.  The executor is designed to have a
sensible external function interface; for more complicated
commands this quite possibly involves a structure or an abstract
type plus accessor functions.  The parser parses a specification
and then calls the executor using its function interface.  If the
parser and the executor are well designed, then they can even be
in completely separate source files; it might even make sense in
some cases to move the executor out of src/language into
src/data, etc.

The code you're proposing divides this at a high level: command.c
calls a function that parses the command and returns a
specification, then calls a function to execute the
specification.  My question is this: is it valuable to push this
division up to this higher level?  As is, I don't see the
advantage of doing it that way.

There is also an objection that calling a function is a fairly
easy and flexible thing to do, that requires only a single line
of code, doesn't require any dynamic memory allocation, and so
on.  But creating a specification as a structure, returning it,
executing it in a separate function, and then discarding it, is
more work and harder to read.  Take the N OF CASES command as a
trivial example.  Before the patch, it looks like this:

    /* Parses the N command. */
    int
    cmd_n_of_cases (struct lexer *lexer, struct dataset *ds)
    {
      /* Value for N. */
      int x;

      if (!lex_force_int (lexer))
        return CMD_FAILURE;
      x = lex_integer (lexer);
      lex_get (lexer);
      if (!lex_match_id (lexer, "ESTIMATED"))
        dict_set_case_limit (dataset_dict (ds), x);

      return lex_end_of_command (lexer);
    }

If you wanted to divide it according to my scheme, it would
become something like this:

    /* Parses the N command. */
    int
    parse_n_of_cases (struct lexer *lexer, struct dataset *ds)
    {
      /* Value for N. */
      int x;

      if (!lex_force_int (lexer))
        return CMD_FAILURE;
      x = lex_integer (lexer);
      lex_get (lexer);
      run_n_of_cases (ds, x, lex_match_id (lexer, "ESTIMATED"));
      return lex_end_of_command (lexer);
    }

    /* Executes the N command, setting the case limit to X. 
       If ESTIMATED is true then X is really just an estimate
       of the number of input cases to expect. */
    int
    run_n_of_cases (struct dataset *ds, int x, bool estimated)
    {
      if (!estimated)
        dict_set_case_limit (dataset_dict (ds), x); 
    }

In your scheme, it's a lot bigger and harder to understand, and
requires casts too:

    struct command_n_of_cases 
      {
        struct command parent;
        int n ; /* Value of N */
      };

    void
    destroy_cmd (struct command *cmd)
    {
      free (cmd);
    }

    static int
    run_n_of_cases (const struct command *cmd_, struct dataset *ds)
    {
      const struct command_n_of_cases *cmd = (const struct command_n_of_cases 
*) cmd_;
      dict_set_case_limit (dataset_dict (ds), cmd->n);

      return CMD_SUCCESS;
    }



    /* Parses the N command. */
    int
    cmd_n_of_cases (struct lexer *lexer, struct dataset *ds UNUSED, struct 
command **cmd)
    {
      struct command_n_of_cases **cmdn = (struct command_n_of_cases**)cmd;
      if (!lex_force_int (lexer))
        return CMD_FAILURE;

      *cmd = xmalloc (sizeof (struct command_n_of_cases));
      (*cmd)->run = run_n_of_cases;
      (*cmd)->destroy = destroy_cmd;
      (*cmdn)->n = lex_integer (lexer);
      lex_get (lexer);

      return lex_end_of_command (lexer);
    }

Of course, for more complicated commands the difference will be
less dramatic.

This is all predicated on the assumption that the GUI should
execute commands directly, without going through the lexer and
syntax parser.  Another alternative would be to make the GUI
generate syntax and feed it to a lexer.  I think that this is a
serious alternative.  SPSS can output syntax for commands
executed through the GUI, which allows users who aren't familiar
with syntax to construct simple syntax files and re-execute them
with simple variations.  I think that this is an important and
useful feature.  If we go this direction, it might be wasted
effort to divide up command implementations into parsing and
execution.
-- 
"doe not call up Any that you can not put downe."
--H. P. Lovecraft