Re: [libgpiod][PATCH v2 3/3] bindings: cxx: implement C++ bindings for libgpiod v2.0

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Thu, Jul 22, 2021 at 12:49 PM Jack Winch <sunt.un.morcov@xxxxxxxxx> wrote:
>
> Hello Bartosz et al.,
>
> Apologies for blowing hot and cold with regard to my contributions and
> responses on this mailing list.  The last nine months have been an
> incredibly busy time.
>
> I would recommend using enum classes, as this offers better type
> safety.  Best practice is to avoid the use of unscoped enums within
> C++, especially of the anonymous variety.  There is now a set of ISO
> C++ Core Guidelines (see
> https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#main)
> which offer sage advice for developing C++ software.  Just be aware
> that one-size doesn't fit all and trying to maintain shared library
> ABI compatibility would be considered an edge case which affects the
> applicability of some guidance.  In time, maybe someone will
> explicitly state which guidelines are exempt in a situation similar to
> our own (i.e., providing ABI stable C++ interfaces to shared libraries
> for major revisions).
>
> ABI compatibility issues can be avoided by using one of the fixed
> width integer types introduced in C++11 (e.g., std::uint8_t) as the
> underlying type of the scoped enum class.
>
> I would personally avoid adding the 'get_' prefix to 'getter' methods
> of classes, although this choice is almost entirely aesthetical in
> nature.
>
> My advice would be to remove all anonymous unscoped enum definitions
> from class definitions, instead defining them as scoped enum classes
> using a suitable fixed-width integer type (i.e., std::uint8_t) outside
> the dependent classes.  Then, within the definitions of dependent
> classes, you can define publicly scoped type aliases for these enum
> class types (using the suffix '_type' for these aliases, as is
> commonly seen with other C++ libraries).  See my rough example below
> for illustrative purposes only.  This is a familiar approach and will
> rectify your current issue with naming collisions.
>
> namespace gpiod {
>
> enum class direction : std::uint8_t {
>     INPUT = 1,
>     OUTPUT
> };
>
> class line_info {
> public:
>     using direction_type = ::gpiod::direction;
>
>     direction_type direction () const {
>         return _m_direction;
>     }
>
> private:
>     direction_type _m_direction;
> };
>
> }
>
> If time permits, I'll do a proper review of your changes (however, I
> emigrate in less than a week, so I'm still rather busy at the moment).
>
> Best,
> Jack

Oh, nice, wish you all the best!

Is there any disadvantage to declaring the scoped class enums as
subtypes within the concerned classes?

I was thinking of:

class line_info
{
public
    class direction
    {
        INPUT = 1,
        OUTPUT,
    };

    direction get_direction(void) const;
};

I also don't like get_<property> style methods but IMO

direction get_direction(void) const;

is better than

direction_type direction(void) const;

Thanks for the link, I'll try to find some time to read it.

Bartosz