From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59831) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uq4O0-00006I-DA for qemu-devel@nongnu.org; Fri, 21 Jun 2013 12:40:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uq4Nz-0006SC-A8 for qemu-devel@nongnu.org; Fri, 21 Jun 2013 12:40:32 -0400 Received: from mail-ie0-x22e.google.com ([2607:f8b0:4001:c03::22e]:36120) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uq4Nz-0006Rn-5p for qemu-devel@nongnu.org; Fri, 21 Jun 2013 12:40:31 -0400 Received: by mail-ie0-f174.google.com with SMTP id 9so19157315iec.5 for ; Fri, 21 Jun 2013 09:40:28 -0700 (PDT) Sender: fluxion Date: Fri, 21 Jun 2013 11:39:57 -0500 From: mdroth Message-ID: <20130621163957.GD12685@vm> References: <1371659287-14331-1-git-send-email-kwolf@redhat.com> <1371659287-14331-2-git-send-email-kwolf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1371659287-14331-2-git-send-email-kwolf@redhat.com> Subject: Re: [Qemu-devel] [PATCH 1/3] qapi.py: Move common code to evaluate() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: lcapitulino@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, armbru@redhat.com 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 > --- > 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 > >