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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux