Mailing-List: contact cygwin-help AT sourceware DOT cygnus DOT com; run by ezmlm
List-Subscribe: <mailto:cygwin-subscribe AT sources DOT redhat DOT com>
List-Archive: <http://sources.redhat.com/ml/cygwin/>
List-Post: <mailto:cygwin AT sources DOT redhat DOT com>
List-Help: <mailto:cygwin-help AT sources DOT redhat DOT com>, <http://sources.redhat.com/ml/#faqs>
Sender: cygwin-owner AT sources DOT redhat DOT com
Delivered-To: mailing list cygwin AT sources DOT redhat DOT com
Date: Mon, 10 Sep 2001 16:26:33 -0400
From: Christopher Faylor <cgf AT redhat DOT com>
To: cygwin AT cygwin DOT com
Subject: Re: *.cwp file recognistion patch for setup
Message-ID: <20010910162633.A17124@redhat.com>
Reply-To: cygwin AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
References: <999956948 DOT 9501 DOT 93 DOT camel AT lifelesswks> <20010908132133 DOT B12465 AT redhat DOT com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20010908132133.B12465@redhat.com>
User-Agent: Mutt/1.3.21i

Oops.  I just realized that I implied that Warren should send email to
cygwin-patches and we've been responding in cygwin instead.

Sorry about that.

cgf

On Sat, Sep 08, 2001 at 01:21:33PM -0400, Christopher Faylor wrote:
>On Sat, Sep 08, 2001 at 11:49:07PM +1000, Robert Collins wrote:
>>2) Your code is very C-like.  That is, it doesn't make use of some the
>>nice things C++ can do for us.  See my suggested solution below for
>>somewhat more detailed approach.
>
>I agree that the patch is C-like and I'd like to see it more C++
>oriented but I wanted to point out (and I'm sure that Chuck agrees with
>me) that setup itself is a strange mishmash of C/C++.  I'd like to
>move away from the C-like nature of setup, though.  So if we are
>implementing entirely new interfaces, it is a good idea to use
>things like classes and methods rather than functions.
>
>>3) You've got magic numbers in the code (*groan*). By this I mean that
>>there are constant strings/numbers (ie gz, bz2, cwp, BZh) in the code
>>with a) no explanation (and while it's trivial code at the moment, these
>>things have habit of growing, and before we know it they are in much
>>less readable code :}) and b) not defined for reuse. A better way to
>>code that sort of thing is via #defines or enums. (ie
>>#define GZ_EXT ".gz"
>>#define BZ2_EXT ".bz2"
>>#define CWP_EXT ".cwp"
>
>Personally, I don't have a problem with using the strings in code since
>it is obvious what you mean by ".tar.bz2" and if you do a
>strncmp (ac, ".tar.bz2", 8), you have a better chance of understanding
>that the 8 is the length of ".tar.bz2".  Putting the ".tar.bz2" in a
>define and then using a strncmp (ac, TAR_BZ2, 8) eliminates that, although,
>hmm, I guess this could also be written as
>strncmp (ac, TAR_BZ2, sizeof (TAR_BZ2) - 1) .
>
>As far as non-obvious strings, like magic numbers are concerned, I agree
>100% that they should be put in a #define or constant string.
>
>That's my 2c.  I agree with everything else that Robert said.
>
>Thanks for taking the time to review and comment on this, Robert.
>
>cgf
>
>--
>Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
>Bug reporting:         http://cygwin.com/bugs.html
>Documentation:         http://cygwin.com/docs.html
>FAQ:                   http://cygwin.com/faq/

-- 
cgf AT cygnus DOT com                        Red Hat, Inc.
http://sources.redhat.com/            http://www.redhat.com/

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/