Re: [PATCH] RFC: CODING_STYLE: describe "self" variable convention

["Marc-André Lureau" <marcandre.lureau@gmail.com>]

Hi

On Wed, Nov 20, 2019 at 9:05 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Nov 20, 2019 at 04:54:44PM +0400, Marc-André Lureau wrote:
> > Following the discussion in thread "[PATCH v3 13/33] serial: start
> > making SerialMM a sysbus device", I'd like to recommend the usage of
> > "self" variable to reference to the OOP-style method instance, as
> > commonly done in various languages and in GObject world.
>
> Looking at glib codebase, I don't see 'self' used all that

Only gio in glib actually uses gobject. You would have to look at
other GNOME C projects to realize this is the most common pattern.

> widely or consistently - much of gio/ uses an abbreviation
> of the object type as the variable name.
>
> >
> > Cc: Peter Maydell <peter.maydell@linaro.org>
> > Cc: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  CODING_STYLE.rst | 38 ++++++++++++++++++++++++++++++++------
> >  1 file changed, 32 insertions(+), 6 deletions(-)
> >
> > diff --git a/CODING_STYLE.rst b/CODING_STYLE.rst
> > index 427699e0e4..cb6635af71 100644
> > --- a/CODING_STYLE.rst
> > +++ b/CODING_STYLE.rst
> > @@ -102,12 +102,38 @@ Rationale:
> >  Naming
> >  ======
> >
> > -Variables are lower_case_with_underscores; easy to type and read.  Structured
> > -type names are in CamelCase; harder to type but standing out.  Enum type
> > -names and function type names should also be in CamelCase.  Scalar type
> > -names are lower_case_with_underscores_ending_with_a_t, like the POSIX
> > -uint64_t and family.  Note that this last convention contradicts POSIX
> > -and is therefore likely to be changed.
> > +Variables are lower_case_with_underscores; easy to type and read.
> > +
> > +The most common naming for a variable is an abbreviation of the type
> > +name.  Some common examples:
> > +
> > +.. code-block:: c
> > +
> > +    Object *obj;
> > +    QVirtioSCSI *scsi;
> > +    SerialMM *smm;
> > +
> > +When writing QOM/OOP-style function, a "self" variable allows to refer
> > +without ambiguity to the instance of the method that is being
> > +implemented (this is not very common in QEMU code base, but it is
> > +often a good option to increase the readability and consistency,
> > +making further refactoring easier as well).  Example:
>
> For me the first "sniff test" for a new coding style guideline is
> whether QEMU actually follows the rule to any significant extent
> already. If not, then I think the benefit would have to be very
> significant to justify defining it as a rule. We've historically

It's not a strict rule.

> be quite reluctant to do bulk updates of existing code to apply
> new coding styles. Without planning a bulk update, you end up
> with a coding style that is followed by 1% of the code and ignored
> by the other 99%.

We won't do a bulk update (that was never my intention, that would be
ridiculous).

Adding "self" to the zoo of variable names to refer to the
implentation instance isn't going to make a revolution, but is a good
pattern. I didn't think we would need to argue about it or modify
CODING_STYLE. But I should have known better ;)

>
> As noted above, the common case in QEMU is for the variable to be an
> abbreviation of the type name. The number of places using "*self" is
> almost single digits. So I think the idea of standardizing on "self"
> is already questionable for QEMU.
>
>
> I think the reason for the current pattern of abbreviated type name
> is that when we're inventing OOP features in C the impl of inheritance
> is always sub-optimal, such that you frequently find a need to cast to
> parent types.  So in any single method it is common to have multiple
> variables all refering to the object "self", each cast to a different
> type. To pick one simple example
>
>     QIOChannelFile fioc = qio_channel_file_new(...)
>     QIOChannel *ioc = QIO_CHANNEL(fioc)
>     Object *obj = OBJECT(fioc)
>
>     qio_channel_read_all(ioc, buf, len, erro);
>     object_unref(obj);

This code is probably not a OOP-style "method" (a method associated to
an instnace). I can't imagine what would be "self" here.

>
>
> I think that using the object type abbreviation for the variable name
> is a good thing.  Arbitrarily picking "self" for one of those variables
> is unhelpful, as you have to then look back to the declaration of "self"
> to remind yourself whether "self" is an QOIChannelFile or a QIOChannel
> or an Object.

Is "s" or "ss" or "ioc", "fioc" more readable?

self is of the type being implemented. Usually it is inside a
my_foo_method() in my-foo.c, so you know that self is of MyFoo type
without effort.

>
> Constrast with C++ / Java, where I think the use of "self" is a good
> thing, because the language has built-in  OOP concepts, such that
> you can call a method on any parent class without having to first
> cast "self" to the parent type. IOW, in those languages you don't
> have to care about the particular types in the class hierarchy when
> operating on "self".  This isn't true of C / QEMU's QOM.
>
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>
>


-- 
Marc-André Lureau