lmi
[Top][All Lists]
Advanced

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

Re: [lmi] lmi tests under cygwin


From: Greg Chicares
Subject: Re: [lmi] lmi tests under cygwin
Date: Wed, 02 Nov 2005 23:02:22 +0000
User-agent: Mozilla Thunderbird 1.0.2 (Windows/20050317)

On 2005-11-2 22:09 UTC, Vadim Zeitlin wrote:
> On Wed, 02 Nov 2005 19:23:47 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> Here's another idea: in 'alert_cli.cpp',
> GC> 
> GC> [Move this thing out of the anonymouse namespace:]
> GC> 
> GC> - namespace
> GC> - {
> GC>       bool ensure_setup = set_alert_functions
> GC>           (status_alert
> GC>           ,warning_alert
> GC>           ,hobsons_choice_alert
> GC>           ,fatal_error_alert
> GC>           );
> GC> + namespace
> GC> + {
> GC> 
> GC> and then somewhere in 'alert.cpp', do something like
> GC> 
> GC>   extern bool ensure_setup;
> GC>   &ensure_setup;
> GC> 
> GC> to see whether that prevents the linker from discarding the first file.
> 
>  Yes, this should work. It's the same idea as used by the macros I wrote
> about except that they use a dummy function (but a dummy variable works
> just as well) while here we have a "real" one we can use already.

If it actually does work for you, let me know and I'll do it in cvs.

>  OTOH one can wonder if the above is really any better than just having
> a set_default_alert_functions(void) function and calling it from alert.cpp?
> So maybe the simplest would be just do it like this?

'alert.hpp' declares a set_alert_functions() that takes four function-pointer
arguments. How would the implementation know which ones to use, for wx as
opposed to cli? I don't think the linker can figure it out when 'alert.cpp'
is in a shared library and 'alert_[wx|cli|cgi].cpp' is in the application;
but that's exactly where we want those things to reside.

It is a defect of the system, IMO, that someone with your experience can't
easily find this explained in the code. I don't see it clearly documented in
'alert.hpp', but it is explained in 'progress_meter.hpp' (where you would
not reasonably expect to find it):

/// The nonmember functions support a particular artifice whose
/// intention is to let a shared library use only this base class,
/// while the application provides a derived-class implementation that
/// is switchable at link time. Thus, a command-line-interface program
/// might link 'progress_meter_cli.o', while gui programs would link an
/// object compiled from some gui implementation. The artifice that
/// accomplishes this in the present implementation is a callback
/// function pointer. The support functions are nonmembers because of
/// Meyer's reasoning in his well-known paper
///   "How Non-Member Functions Improve Encapsulation"

That's yet another reason for carefully abstracting the technique,
implementing and documenting it in one place, and then reusing it
everywhere applicable. A system whose design is neither obvious, nor
documented in an obvious place, is unmaintainable. A system that can
be understood only by its author is unmaintainable.

Forensic code review would suggest (and my knowledge of the history
confirms) that 'alert.hpp' was written first, then 'progress_meter.hpp',
then 'callback.hpp'--and the documentation has improved with every
evolutionary step, as you can see by reading the first paragraph of
the 'callback.hpp' documentation:

/// Design notes for class callback.
///
/// This class encapsulates management of a callback function across a
/// shared-library boundary. The shared library calls the function
/// through a pointer. Another module implements the function. This
/// class helps ensure that the pointer is initialized correctly
/// whenever it's used.

To merge all this stuff, retrofitting the 'callback' library to the
other two facilities that should use it, is delicate work. One must
understand all three fully, then consider each of their differences,
in terms of documentation (as above) and code as well, e.g. the
difference in 'volatile' qualifiers here:

  progress_meter_wx.cpp:
    volatile bool ensure_setup = set_progress_meter_creator
        (concrete_progress_meter_creator
        );

  alert_wx.cpp
    bool ensure_setup = set_alert_functions
        (four arguments...

To replace 'callback' with boost::function is similarly delicate
work. One can write the code and inline documentation to make the
design clear, or spend a similar or greater amount of time later on
discussing it; the former is much better. Striving for excellence
is the least expensive way in the long term, but in the short term
it costs more without delivering the immediate benefits that those
who pay us desire. Therefore, until we can hire more programmers
who care about excellence, I have to focus my efforts narrowly, and
not try to perfect everything at once; and this matter can wait.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]