Skip to main content
2 of 3
added 133 characters in body
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341

All my comments are very minor:

###Dry Code: Your constructor/assignment operator are not DRY.

constexpr explicit gray_code(value_type value) noexcept:
    value( (value >> 1) ^ value )
{}

auto operator=(value_type other) & noexcept
    -> gray_code&
{
    value = (other >> 1) ^ other;
    return *this;
}

If things change you have to modify the code in multiple places. You should wrap the work in a function. That both these methods use:

constexpr explicit gray_code(value_type value) noexcept:
    value( convertValueToGrey(value) )
{}

auto operator=(value_type other) & noexcept
    -> gray_code&
{
    value = convertValueToGrey(other);
    return *this;
}

###New Return Type: Not sure I like the new style of return:
I personal don't think it is as clear (but that may be me and I need to get used to it). Personally I currently only use the new style when the return type is being derived.

auto operator=(value_type other) & noexcept
    -> gray_code&

When not deriving the return type I still prefer the old style.

gray_code& operator=(value_type other) & noexcept

But this is not a big deal. The project/company style guide will dictate what is prefered.

###Public Member Variables: Your value is public:
Not sure if this is on purpose? You have assignment that does conversion on the way in and a conversion operator to adjust the value on the way out. Do you really want to allow access to the raw value for manipulation without supervision?

struct gray_code
{
    value_type value;

###Costly Conversion The conversion back to value_type seems a bit heavy:

operator value_type() const noexcept
{
    value_type res = value;
    for (value_type mask = std::numeric_limits<value_type>::digits / 2
         ; mask ; mask >>= 1)
    {
        res ^= res >> mask;
    }
    return res;
}

If you are doing this more than a couple of times that may become expensive. Why not pre-prepare it and store it in a mutable member.

// Variable containing the gray code
value_type          value;
mutable value_type  convertedValue; // Created on construction/assignment.
                                    // Marked as mutable to indicate that
                                    // it is not part of the state of the
                                    // object but rather a cached value.

operator value_type() const noexcept  {return convertedValue;}

Personally I think your code is a little heavy on useless comments.

Loki Astari
  • 97.7k
  • 5
  • 126
  • 341