Re: [PATCH] Choose syntax



Hi Roland,

On Tue, 2005-07-12 at 08:41, Roland Illig wrote:
> exec_syntax_dialog should not use the constant MAX_SYNTAX_FILES, but 
> some parameter names_size. That makes the relation between them tight 
> closer.

As I've explained the caller cannot make any sensible guess about the
size of the list, so apart from creating it dynamically it's easier to
just use a sufficiently large char**. Adding a size parameter as you
propose would only suggest that something sensible can be put into that
parameter and it can not.

However I agree I should change the parameter list of
exec_edit_syntax_dialog to const char *names[MAX_SYNTAX_FILES + 1].

> I meant: _use_ strcmp, but don't use boolean operators with it.

Aha.

> Don't ever use boolean operators if you don't intend them to _mean_ 
> logic manipulation.

Well, as you can see all these are logic manipulations. I use the
!strcmp() to see if the strings match TRUE/FALSE.

> To me it looks quite good, although it does not follow the usual C syntax.

<g>. Double standards I see.

> cvs diff -r1.19 -r1.20 edit/editmenu.c
> 
> It's like I expected it. That was not "common practice" but superfluous 
> from the beginning, hidden inside a whole bunch of functions where the 
> wrapper was actually necessary. But not in that one case.

They might be redundant from a strict functional viewpoint, but from a
consistent naming perspective (menu_* functions in editmenu.c,
edit_options_dialog() in edit_options.c) they are useful. Let them be.

You ask me to add comparison operators for clarity's sake but you ask me
to remove a wrapper that has a similar function (making the code more
readable).

Leonard.

-- 
mount -t life -o ro /dev/dna /genetic/research





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]