Review Request: wetter.com Weather Ion
Thilo-Alexander Ginkel
thilo at ginkel.com
Tue Nov 3 19:51:47 CET 2009
On Tuesday 03 November 2009 18:51:05 Aaron Seigo wrote:
> > On 2009-11-03 17:00:51, Will Stephenson wrote:
> > > /trunk/kdereview/plasma/dataengines/weather/ions/ion_wettercom.cpp,
> > > line 44
> > > <http://reviewboard.kde.org/r/2026/diff/2/?file=13614#file13614line44>
> > >
> > > By convention, public member variables of pimpl classes are not
> > > names with m_ prefixes - since accessing them via d-> in the outer
> > > class makes it obvious that they belong to the pimpl
>
> this isn't public API. why does it have a pimpl at all? i think some people
> have missed the point of the FooPrivate classes: maintaining binary
> compatibility in publicly exported APIs. for anything else they are just a
> bit more overhead, one more level of indirection and one more opportunity
> for a memory leak.
I chose this approach as that's what the existing Weather Ions are using, but
also agree that this is not a public API, so binary compatibility is a non-
issue. If you prefer, I can easily remove the pimpl.
Regards,
Thilo
More information about the Plasma-devel
mailing list