Am 21.06.2013 um 18:39 hat mdroth geschrieben:
> 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 <kw...@redhat.com>
> > ---
> > 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...)
As if it made any difference if it's one level more or less... It's
already buried in the "library".
> 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()?
I don't think there's any value in keeping the global stuff in
parse_schema() (or, to be precise, in functions directly called by
parse_schema()) , and I found it quite nice to have evaluate()
actually evaluate something instead of just parsing it.
But I agree that duplicating code, as small as it may be now, isn't
nice, so I can try to use the get_expr() thing. I just would prefer to
move the evaluation to evaluate() anyway.
Kevin
