From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12524 invoked by alias); 23 Oct 2002 19:47:44 -0000 Mailing-List: contact rhdb-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Post: List-Help: , Sender: rhdb-owner@sources.redhat.com Received: (qmail 12517 invoked from network); 23 Oct 2002 19:47:43 -0000 Received: from unknown (HELO touchme.toronto.redhat.com) (216.138.202.10) by sources.redhat.com with SMTP; 23 Oct 2002 19:47:43 -0000 Received: from redhat.com (totem.toronto.redhat.com [172.16.14.242]) by touchme.toronto.redhat.com (Postfix) with ESMTP id E111680007A; Wed, 23 Oct 2002 15:47:42 -0400 (EDT) Message-ID: <3DB6FCDE.2030901@redhat.com> Date: Wed, 23 Oct 2002 12:47:00 -0000 From: Fernando Nasser Organization: Red Hat , Inc. - Toronto User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1) Gecko/20020827 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Brett Schwarz Cc: rhdb@sources.redhat.com Subject: Re: rhdb-admin References: <1035397212.6808.92.camel@thor> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2002-q4/txt/msg00004.txt.bz2 Brett Schwarz wrote: > No patches, but some recommendations: > Thanks for the hints and for taking the time to look into it. > 1) Whenever you use 'expr', you should enclose the args in curly > brackets: > > expr 1 + 2 should be: expr {1 +2} > > Same goes for 'if' statements, although I didn't explicitly see any... > > This prevents double substitution (i.e. double processing). Not a big > thing, but if there are alot of them, it may add up in the performance. > This should work in most of the cases (there are some outliners). > All the if statements should have the conditions between braces as it is the coding style used. > 2) To load Itcl, Itk, and Iwidgets, you only need to do package require > Iwidgets ... it loads Itk, and Itcl as well... > Any harm in leaving them there? The packages should all be loaded very early in the execution anyway and having the explicit require better documents things. Besides we may depend on a specific version of some of these packages one day. > 3) You also gain some by packing widgets together that have the same > pack options. > I do have the feeling that we have too many packages. Maybe it is a good idea to simplify this for next version. > 4) In some of the classes, the Itcl commands are not fully qualified, > and rhdb-admin doesn't even startup. I just added namespace import > ::itcl::* to make it work, but maybe a better idea is to fully qualify > the Itcl commands, unless you know for sure there won't be any name > clashing. (I think the one I remember was configbody in > NewDisjointListBoxWidget class) > We fixed this already and will be checking this and a few other things in cvs soon. I run into this problem myself when I upgraded my version of itcl/iwidgets. > I haven't gone through thoroughly yet, but looks good... > Thanks! > HTH, > > --brett > > p.s. more performance tips here: http://mini.net/tcl/348 > > Very interesting. Thanks for the pointer. Regards, Fernando -- Fernando Nasser Red Hat - Toronto E-Mail: fnasser@redhat.com 2323 Yonge Street, Suite #300 Toronto, Ontario M4P 2C9