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...) 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 > >