[libcamera-devel] [PATCH v2 2/5] libcamera: Add a base class to implement the d-pointer design pattern

[Jacopo Mondi]

Hi Laurent

On Tue, Oct 27, 2020 at 02:19:15PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Tue, Oct 27, 2020 at 01:11:06PM +0100, Jacopo Mondi wrote:

[snip]

> > > > In private class:
> > > >         ClassName *const asdsad = _public<ClassName>();
> > > >
> > > > without introducing macros that has to be followed and really just
> > > > wrap one method call.
> > >
> > > The variable name isn't constrained by this patch, unlike v1 that hid
> > > the variable declaration in the macro. I however want to keep the names
> > > consistent, as that increases readability of the code base. Anyone
> > > familiar with libcamera should be able to immediately understand what d
> > > and o are (d comes from the d-pointer design pattern and o stands for
> > > object, but I'm fine discussing different names, especially for o),
> > > hence patch 1/5 in this series to enforce that rule.
> >
> > Yeah, I meant not reporting an error in checkstyle.
> >
> > > As for calling functions directly instead of using macros, I think the
> > > macros make it more readable by hiding the implementation details.
> >
> > Ok, so, and sorry for backtracking, if you want 'd' and 'o' to be
> > forced and get it as recognizible libcamera construct, macros are fine
> > too
>
> Just to be sure, does this mean you prefer
>
> 	Private * const d = LIBCAMERA_D_PTR();
>
> or
>
> 	LIBCAMERA_D_PTR();
>
> with the d variable hidden in the macro ?

I would prefer letting each implementation use the variable name they
like, without introducing any library specific pattern that increase
the entry barrier to grasp the code base.

But I see your point and if you want to force the usage of the 'o' and
'd' variable names using checkstyle, I think the two are equivalent,
with a slight preference for the first that at least has the variable
name explicit and not hidden behind a macro.

Up to you, really.