Re: [Plplot-devel] Qt driver problems/suggestions (fwd)

[Alban R. <a.r...@im...>]

Hello all,

I've just reviewed Dmitri's patch, and I am OK with it, as most of it is
"aesthetic", and it fixes a couple of mistakes (e.g. non-virtual
destructor on class inheriting QObject). This code grew very organically
from out base from QSAS, where we don't have a clearly defined coding
style, and I admit I didn't pay much attention to that, so why not
choose Dmitri's :)

The only thing I would reproach in it is that although it arranged the
brackets/spaces/etc., it messed up the indenting. I chose to use tabs
rather than spaces, and I'm ok to change for the other way, but it's not
consistent any more. Anyway, these are only very minor considerations.

Although I will not have much time in the future for the maintenance of
this driver - and not at all before mid-July - I plan a new update by
the end of July, in which I will improve a few things, so I can
incorporate any suggestion as long as it is not overwhelming. I will see
how I can re-think the static objects issue. As for splitting qt.h into
as many headers as there are classes, I chose not to do so to keep the
"drivers" directory easier to read (1 header for the Qt drivers rather
than 7), but I admit this is very debatable, so feel free to change that
if you wish.

Alban

Alan W. Irwin wrote:
> Hi Alban (off list):
>
> Could you respond on the plplot-devel mailing list to Dmitri's patch he just
> sent to that list concerning the coding style of the qt device driver code?
> I am afraid it is quite possible the small changes I introduced to the qt
> device driver have added some style issues because of my lack of C++
> expertise.  Of course, the best coding style is a matter of opinion, but
> ideally you and Dmitri will be able to come to consensus on best style.
> However, if there is a difference of opinion, I will follow yours.
> Furthermore, I would be happy to test whatever style changes come out of
> this discussion to make sure no regressions have been introduced by the
> changes.
>
> To give you some more background on this style issue, I have forwarded below
> what Dmitri sent me privately a few days ago.  It includes some background
> information on his choice of preferred coding style that he unfortunately
> did not send to the list with his patch.  I responded to his questions by
> letting him know I would defer to your judgement which I am doing now.  :-)
>
> Best wishes,
>
> Alan
> __________________________
> Alan W. Irwin
>
> Astronomical research affiliation with Department of Physics and Astronomy,
> University of Victoria (astrowww.phys.uvic.ca).
>
> Programming affiliations with the FreeEOS equation-of-state implementation
> for stellar interiors (freeeos.sf.net); PLplot scientific plotting software
> package (plplot.org); the libLASi project (unifont.org/lasi); the Loads of
> Linux Links project (loll.sf.net); and the Linux Brochure Project
> (lbproject.sf.net).
> __________________________
>
> Linux-powered Science
> __________________________
>
> ---------- Forwarded message ----------
> Date: Fri, 19 Jun 2009 20:15:52 +0300
> From: Dmitri Gribenko <gri...@gm...>
> To: Alan W. Irwin <ir...@be...>
> Subject: Qt driver problems/suggestions
>
> Hello Alan,
>
> After looking at the source, I found the following problems:
>
> 1. Inconsistent coding style, long hard-to-read expressions without
> spacing around operators, c-style casts etc.  I make it conform more
> or less to Google C++ coding guidelines [1].
>
> 2. Code uses two static global objects derived from QObject
> (QtPLDriver::mutex and an instance of MasterHandler).  This should be
> avoided because construction order is not defined.  Also, on my system
> this leads to destructors of those objects being called two times.
> Anyway, Qt and KDE developers say this is a bad thing [2] [3].
> Although KDE developers offer some sort of a macro that could mitigate
> those problems, I think we could come up with a solution that creates
> the object during plplot init and de-init.
>
> 3. qt.h has some things (for example, MasterHandler) that user's code
> doesn't need to see.  Also, to go along with Qt programming style, one
> should place only one class declaration per header.
>
> I can try to fix it, but I know that Qt driver is under active
> development so I decided to ask you first (maybe you have some local
> copy that differs much from what is in svn).
>
> [1] http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml
> [2] http://qt.gitorious.org/qt/pages/CodingConventions
> [3] http://techbase.kde.org/Policies/Library_Code_Policy#Static_Objects
>
>
>