[Spice-devel] [PATCH spice-gtk 2/5] widget: minor code style improvements

Christophe Fergeau cfergeau at redhat.com
Thu Jun 8 08:57:30 UTC 2017


On Thu, Jun 08, 2017 at 04:41:55AM -0400, Marc-André Lureau wrote:
> Hi
> 
> ----- Original Message -----
> > On Thu, Jun 08, 2017 at 04:31:20AM -0400, Marc-André Lureau wrote:
> > > Hi
> > > 
> > > ----- Original Message -----
> > > > On Thu, Jun 08, 2017 at 12:55:38AM +0400, marcandre.lureau at redhat.com
> > > > wrote:
> > > > > From: Marc-André Lureau <marcandre.lureau at redhat.com>
> > > > > 
> > > > > Use shorter line, use the common "d" variable for private data access,
> > > > > add brackets to ease reading the inner block vs the condition, remove
> > > > > needless != NULL.
> > > > 
> > > > I'd lean towards NACK for this one, one letter variable names is imo
> > > > very bad for readability. I know this is widespread in the spice-gtk
> > > > codebase, but I'd at least rather not expand that usage.
> > > > 
> > > 
> > > You may rename it "priv", but then you lost the benefit of being really
> > > short.
> > 
> > Being really short is a benefit? This is where we are going to disagree :)
> 
> Well, in an proper OO language, you wouldn't even have it, it would be like magic!

Maybe, maybe not, 'priv' members in C++ classes are not that unusual
( https://en.wikipedia.org/wiki/Opaque_pointer#C.2B.2B )

> So yes, I like private member being accessed with a very short
> variable in C. If it's use consistently, there is very little
> confusion possible imho.

As I said, we are not going to agree there :) "If it's used
consistently, it does not cause confusion", does not mean that's a good
thing :) This just makes things harder to read for someone not knowing
the convention, and for no great reason (saving at most a few seconds of
typing?)

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170608/2337e71d/attachment.sig>


More information about the Spice-devel mailing list