[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] RFC: CODING_STYLE: describe "self" variable convention
From: |
Marc-André Lureau |
Subject: |
Re: [PATCH] RFC: CODING_STYLE: describe "self" variable convention |
Date: |
Wed, 20 Nov 2019 21:37:20 +0400 |
Hi
On Wed, Nov 20, 2019 at 9:05 PM Daniel P. Berrangé <address@hidden> 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 <address@hidden>
> > Cc: Daniel P. Berrangé <address@hidden>
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> > 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