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

gEDA-dev: Good software engineering practices for gEDA



On Sunday 27 May 2007 19:00:08 Ivan Stankovic wrote:

> > I'm uneasy about the use of "atexit(..)" with regards to portability. It
> > seems a little low level for us to be using generally like this.
>
> You shouldn't worry about portability, atexit() is required by C89,
> C99 and POSIX standards. The only problem might be if the code using it
> is dependent on the order in which other on-exit functions are called,
> which is clearly not the case here. I actually think using atexit is
> quite elegant as it avoids the need to have yet another global function
> that should be called on program exit.
>
> > I'm unsure where the correct place to call from is, but x_window_close()
> > or gschem_quit() look like contenders. We could add a close handler
> > hooking API for this so that we don't have to explicitly add a
> > non-static save_geometry_to_file(..) and call it ourselves.
>
> This is a possibility, but what would be the advantage over atexit?
>

You could make some Scheme interface to it, so you could have arbitrary Scheme 
code called on exit.

(gschem-exit-hook (lambda () (display "I'm an exit hook!")))

It would be very nice to have a general mechanism for hooking onto events like 
exiting gschem, in case it's needed for work in the future.  In addition, 
it's very clear to the person reading the code *exactly* when and where the 
hook will be called, which is actually very important.

Ivan, I'm a little bit troubled by the code you've been submitting.  Although 
it clearly works -- and works well -- and there's nothing technically *wrong* 
about it, some of the things you've done seem a bit hackish.

For instance, gschem didn't support dynamic menus declared using the Scheme 
extension mechanism, but you needed this for the recent files menu.

 - Nice way of doing it: add support for dynamic menus directly to the
   menu-generating mechanism by registering Guile procedure callback
   functions, changing the way it works if necessary.  Then use new features
   to implement recent file lists.  (And then a "Pages" menu, "Recent
   Symbols", etc, etc).

 - Your way of doing it: use a magic string in menu list, which is magically
   substituted in the C code.  It works, but it works for that feature and
   only that feature.

People who understand the gEDA code base are few -- I don't even pretend to 
know how everything works outside my areas of interest -- so developer time 
is at an absolute premium.  Making sure that current developers' time is well 
spent, and attracting new developers to the project, both require the code 
base to evolve in ways that ensure that not only does code do what you expect 
it to, but also that new features and capabilities added are designed in as 
generally useful way as sensibly possible.

I've tried hard to work in this way in my work on the component library, and 
this has meant that I've been able to add a lot of quite cool new features 
with special cases very well contained, and that I now have a clear roadmap 
for even more awesome things I can put in with a minimum of new code needing 
to be written (in fact, hopefully I'll be able to *remove* some of the code 
from libgeda's C source).

Please don't take this criticism to personally; it's something I think many 
people need to think about, not just you (or me).  It's been brewing in my 
mind for the last few weeks, and has only really bubbled to the surface now.

Regards,

Peter

-- 
Fisher Society                              http://tinyurl.com/o39w2
CU Small-Bore Club                          http://tinyurl.com/mwrc9

      09f911029d74e35bd84156c5635688c0            peter-b.co.uk

PGP signature



_______________________________________________
geda-dev mailing list
geda-dev@moria.seul.org
http://www.seul.org/cgi-bin/mailman/listinfo/geda-dev