Refactoring scoped enum bitwise operator code duplication

Question

I have several scoped enums that can be used as bitwise flags. I've implemented the bitwise operator overloads identically for every type as exampled here:

ScopedEnumFoo& operator|=(ScopedEnumFoo& a, const ScopedEnumFoo& b) noexcept {
    using underlying = std::underlying_type_t<ScopedEnumFoo>;
    auto underlying_a = static_cast<underlying>(a);
    auto underlying_b = static_cast<underlying>(b);
    a = static_cast<ScopedEnumFoo>(underlying_a | underlying_b);
    return a;
}

ScopedEnumFoo operator|(ScopedEnumFoo a, const ScopedEnumFoo& b) noexcept {
    a |= b;
    return a;
}

ScopedEnumFoo& operator&=(ScopedEnumFoo& a, const ScopedEnumFoo& b) noexcept {
    using underlying = std::underlying_type_t<ScopedEnumFoo>;
    auto underlying_a = static_cast<underlying>(a);
    auto underlying_b = static_cast<underlying>(b);
    a = static_cast<ScopedEnumFoo>(underlying_a & underlying_b);
    return a;
}

ScopedEnumFoo operator&(ScopedEnumFoo a, const ScopedEnumFoo& b) noexcept {
    a &= b;
    return a;
}

Other than the scoped enum's type, the code is identical for every type that needs to be used as a flag-ish type. This causes code quality checkers to throw hissy fits that I've duplicated code a dozen (or more) times.

How would I go about "de-duplicating" the code? Is it even possible?

As Jason Turner recently put it:

'I will not copy-paste code' is the single most important thing you can do to write good code...

score 2 · Accepted Answer · answered Jul 15 '20 at 04:29

When you have identical functions operating on different types that is what a template generates so the the first step is to create a template:

template <class E>
E& operator|=(E& a, const E& b) noexcept {
    using underlying = std::underlying_type_t<E>;
    auto underlying_a = static_cast<underlying>(a);
    auto underlying_b = static_cast<underlying>(b);
    a = static_cast<E>(underlying_a | underlying_b);
    return a;
}

The problem with this is that it happily accepts any type and will cause troubles and interfere with other parts of the code in ways often unexpected. I strongly advise against this version, even if the operators are behind a namespace.

So it needs to be restricted to just the wanted types. I will use concepts because they are more expressive. For pre C++20 it's easy to convert to classic SFINAE technique instead.

A quick fix could be to accept just enums:

template <class E>
    requires std::is_enum_v<E>
E& operator|=(E& a, const E& b) noexcept {
    using underlying = std::underlying_type_t<E>;
    auto underlying_a = static_cast<underlying>(a);
    auto underlying_b = static_cast<underlying>(b);
    a = static_cast<E>(underlying_a | underlying_b);
    return a;
}

This is still problematic however as it will accept any enum types, even ones not defined by you. To fix this there are some ways, like adding a sentinel/tag dummy enum value for all your enums, but the method I would chose is to have a trait for your enums (just give it a meaningful name):

template <class E> struct IsAwesomeEnum: std::false_type {};

template <> struct IsAwesomeEnum<ScopedEnumFoo>  : std::true_type {};
template <> struct IsAwesomeEnum<ScopedEnumBar>  : std::true_type {};

template <class E>
    requires IsAwesomeEnum<E>::value
E& operator|=(E& a, const E& b) noexcept {
    using underlying = std::underlying_type_t<E>;
    auto underlying_a = static_cast<underlying>(a);
    auto underlying_b = static_cast<underlying>(b);
    a = static_cast<E>(underlying_a | underlying_b);
    return a;
}

And I would go one step further and define a concept for it:

template <class E>
concept AwesomeEnum = IsAwesomeEnum<E>::value;

template <AwesomeEnum E>
E& operator|=(E& a, const E& b) noexcept {
    using underlying = std::underlying_type_t<E>;
    auto underlying_a = static_cast<underlying>(a);
    auto underlying_b = static_cast<underlying>(b);
    a = static_cast<E>(underlying_a | underlying_b);
    return a;
}

For completeness here is one non-concepts SFINAE approach:

template <class E, class = std::enable_if_t<IsAwesomeEnum<E>::value>>
E& operator|=(E& a, const E& b) noexcept {
    using underlying = std::underlying_type_t<E>;
    auto underlying_a = static_cast<underlying>(a);
    auto underlying_b = static_cast<underlying>(b);
    a = static_cast<E>(underlying_a | underlying_b);
    return a;
}

This is a great answer ( I can't wait for MSVC to fully and natively support concepts). I had to use the SFINAE approach but I was able to put the templates in a utilities header and made it so any scoped enum that declares itself as a bitwise type with: `template<> struct TypeUtils::is_bitflag_enum_type : std::true_type {}; ` automatically gets the appropriate functions generated. I also was able to add versions for incrementability and decrementability. — Casey, Jul 24 '20 at 15:37
[is_scoped_enum](https://en.cppreference.com/w/cpp/types/is_scoped_enum) is getting added in C++23! :D — Casey, May 05 '21 at 06:44
Is there a specific reason (other than way of habit) to take the operand on the right-hand side by `const&` instead of by value? Since the underlying type of enums are integral types, shouldn't taking them by value be the preferred way? What are your thoughts on this? — 303, Dec 21 '21 at 23:25
@303 you are right, it is more efficient to take by value. However these operators will almost sure always be inlined in which case it won't make any difference. So I don't really concern too much with it. — bolov, Dec 21 '21 at 23:45

Refactoring scoped enum bitwise operator code duplication

1 Answers1