Refactoring scoped enum bitwise operator code duplication

83 views Asked by At

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...

1

There are 1 answers

4
bolov On BEST ANSWER

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;
}