Re: [Qemu-devel] [PATCH 1/3] qapi.py: Move common code to evaluate()

[mdroth]

On Wed, Jun 19, 2013 at 06:28:05PM +0200, Kevin Wolf wrote:
> Don't duplicate more code than is really necessary.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  scripts/qapi.py | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 02ad668..3a64769 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -76,12 +76,18 @@ def parse(tokens):
>          return tokens[0], tokens[1:]
> 
>  def evaluate(string):
> -    return parse(map(lambda x: x, tokenize(string)))[0]
> +    expr_eval = parse(map(lambda x: x, tokenize(string)))[0]
> +
> +    if expr_eval.has_key('enum'):
> +        add_enum(expr_eval['enum'])
> +    elif expr_eval.has_key('union'):
> +        add_enum('%sKind' % expr_eval['union'])
> +
> +    return expr_eval

add_enum() adds stuff to a global table, so I don't really like the idea
of pushing it further down the stack (however inconsequential it may be
in this case...)

I think the real reason we have repetition is the extra 'flush' we do
after the for loop below to handle the last expression we read from a
schema, which leads to a repeat of one of the clauses in the loop body.
I've wanted to get rid of it a few times in the past so this is probably
a good opportunity to do so.

Could you try adapting something like the following to keep the
global stuff in parse_schema()?

def get_expr(fp):
    expr = ""

    for line in fp:
        if line.startswith('#') or line == '\n':
            continue
        if line.startswith(' '):
            expr += line
        elif expr:
            yield expr
            expr = line
        else:
            expr += line

    yield expr

def parse_schema(fp):
    exprs = []
    expr_eval

    for expr in get_expr(fp):
        expr_eval = evaluate(expr)
        if expr_eval.has_key('enum'):
            add_enum(expr_eval['enum'])
        ...
                
    return exprs

> 
>  def parse_schema(fp):
>      exprs = []
>      expr = ''
> -    expr_eval = None
> 
>      for line in fp:
>          if line.startswith('#') or line == '\n':
> @@ -90,23 +96,13 @@ def parse_schema(fp):
>          if line.startswith(' '):
>              expr += line
>          elif expr:
> -            expr_eval = evaluate(expr)
> -            if expr_eval.has_key('enum'):
> -                add_enum(expr_eval['enum'])
> -            elif expr_eval.has_key('union'):
> -                add_enum('%sKind' % expr_eval['union'])
> -            exprs.append(expr_eval)
> +            exprs.append(evaluate(expr))
>              expr = line
>          else:
>              expr += line
> 
>      if expr:
> -        expr_eval = evaluate(expr)
> -        if expr_eval.has_key('enum'):
> -            add_enum(expr_eval['enum'])
> -        elif expr_eval.has_key('union'):
> -            add_enum('%sKind' % expr_eval['union'])
> -        exprs.append(expr_eval)
> +        exprs.append(evaluate(expr))
> 
>      return exprs
> 
> -- 
> 1.8.1.4
> 
>