Proposals to change .Not-> operator syntax to .Not()

[twoixter]

Hi @cegonse !! I have some proposals to change the .Not-> syntax for the negation operator. Using pointers and explicit instantiation with new seems weird. It is also error prone since they're not smart pointers and the current implementation does not allow for Not->Not-> chaining.

I have made three alternatives for you to consider. Let me know what do you think!

1) Without pointers

The first alternative is similar to the current one but does not use pointers. This version returns a new instance of the Assertion<T> but negated.

#include <iostream>

template <typename T>
class Assertion {
public:
    Assertion(T value, bool negated = false)
    : value(value), negated(negated)
    {}

    void toBe(T expected)
    {
        if ((value != expected) ^ negated) {
            std::cout << "[FAIL] OOPS!!: [" << value << "] no es [" << expected << "]" << std::endl;
        }
    }

    Assertion<T> Not()
    {
        return Assertion<T>(value, !negated);
    }

protected:
    T value;
    bool negated;
};

template <>
class Assertion<int> {
public:
    Assertion(int value, bool negated = false)
    : value(value), negated(negated)
    {}

    void toBeEven()
    {
        if ((value % 2 != 0) ^ negated) {
            std::cout << "[FAIL] OOPS!!: [" << value << "] no es par" << std::endl;
        }
    }

    Assertion<int> Not()
    {
        return Assertion<int>(value, !negated);
    }

private:
    int value;
    bool negated;
};

int main()
{
    Assertion<std::string> pepito{"pepe"};
    pepito.Not().toBe("manolo");            // OK

    Assertion<int> number{2};
    number.Not().toBeEven();                // FAILS    ==> !value
    number.Not().Not().toBeEven();          // OK       ==> !!value
    number.Not().Not().Not().toBeEven();    // FAILS    ==> !!!value
}

2) Using an AssertionBase class

This is a second attempt trying to DRY and extracting the negated variable for the polarity out of the Assertion class. With an AssertionBase class we can extract this logic into a ensure(...) method that will generalise the negation operation out of the Assertion classes. Having a AssertionBase will also allow to extract general information about the Assertion (file and line perhaps?).

#include <iostream>

class AssertionBase {
public:
    AssertionBase(bool negated = false) : n{negated}
    {}

    bool ensure(bool expression)
    { return expression ^ n; }

protected:
    bool flip_polarity()
    { return !n; }

private:
    bool n;
};

template <typename T>
class Assertion : public AssertionBase {
public:
    Assertion(T value, bool negated = false) : AssertionBase{negated}, value{value}
    {}

    void toBe(T expected)
    {
        if (ensure(value != expected)) {
            std::cout << "[FAIL] OOPS!!: [" << value << "] no es [" << expected << "]" << std::endl;
        }
    }

    Assertion<T> Not()
    { return Assertion<T>{value, flip_polarity()}; }

protected:
    T value;
};

template <>
class Assertion<int> : public AssertionBase {
public:
    Assertion(int value, bool negated = false) : AssertionBase{negated}, value{value}
    {}

    void toBeEven()
    {
        if (ensure(value % 2 != 0)) {
            std::cout << "[FAIL] OOPS!!: [" << value << "] no es par" << std::endl;
        }
    }

    Assertion<int> Not()
    { return Assertion<int>{value, flip_polarity()}; }

private:
    int value;
};

int main()
{
    Assertion<std::string> pepito{"pepe"};
    pepito.Not().toBe("manolo");            // OK

    Assertion<int> number{2};
    number.Not().toBeEven();                // FAILS    ==> !value
    number.Not().Not().toBeEven();          // OK       ==> !!value
    number.Not().Not().Not().toBeEven();    // FAILS    ==> !!!value
}

3) Using template metaprograming

Iterating on the AssertionBase idea, why not giving this base class his own semantics? We can think of two base classes: PositiveAssertion and NegativeAssertion. Using this approach we can get rid of the positive bool variable, since the semantics of the class will allow to know which Assertion we are working on.

We need to switch to two template parameters, though, and use partial template specialisation for custom Assertions. To switch the sign of the Assertion we can leverage type alias so each base assertion will know which is their opposite. (E.G: using reverse_t = NegativeAssertion for the PositiveAssertion when instantiating the Not() class).

#include <iostream>

// Forward declaration of NegativeAssertion to be used later
struct NegativeAssertion;

// Declare two Assertion structs that will enable circular declarations
// of NegativeAssertion -> PositiveAssertion -> NegativeAssertion trough
// the use of the reverse_t alias. It will also hold methods to
// correctly ensure that the Assertion yields the correct expectation.
struct PositiveAssertion {
    using reverse_t = NegativeAssertion;

    bool ensure(bool expression)
    { return expression; }
};

struct NegativeAssertion {
    using reverse_t = PositiveAssertion;

    bool ensure(bool expression)
    { return !expression; }
};

// Assertion base class with a starting PositiveAssertion
template <typename T, typename P = PositiveAssertion>
class Assertion : public P {
public:
    Assertion(T value) : value{value}
    {}

    void toBe(T expected)
    {
        // Call the base "ensure" method. This will be either
        // PossitiveAssertion or NegativeAssertion
        if (P::ensure(value != expected)) {
            std::cout << "[FAIL] OOPS!!: [" << value << "] no es [" << expected << "]" << std::endl;
        }
    }

    // The Not() method will return a new instance of this class
    // but with the Assertion reversed.
    Assertion<T, typename P::reverse_t> Not()
    { return Assertion<T, typename P::reverse_t>{value}; }

protected:
    T value;
};

// To add custom assertions we need to do a partial specialization
// of the Assertion templated class. The P class will be covered
// by the default class type of PossitiveAssertion.
template <typename P>
class Assertion<int, P> : public P {
public:
    Assertion(int value) : value{value}
    {}

    void toBeEven()
    {
        if (P::ensure(value % 2 != 0)) {
            std::cout << "[FAIL] OOPS!!: [" << value << "] no es par" << std::endl;
        }
    }

    Assertion<int, typename P::reverse_t> Not()
    { return Assertion<int, typename P::reverse_t>{value}; }

private:
    int value;
};

int main()
{
    Assertion<std::string> pepito{"pepe"};
    pepito.Not().toBe("manolo");            // OK

    Assertion<int> number{2};
    number.Not().toBeEven();                // FAILS    ==> !value
    number.Not().Not().toBeEven();          // OK       ==> !!value
    number.Not().Not().Not().toBeEven();    // FAILS    ==> !!!value
}

[cegonse]

Wow @twoixter , thank you so much for such a detailed proposal! Following our conversation a couple days ago, I agree this is a much needed improvement over the current form.

I think its worth to fully go for the latest proposal, but step by step.

A first step would be removing the heap allocation and the pointer by implementing the first proposal, and changing the API. Since further improvements to the code will not supose an API break, these can be implemented incrementally.

How would you prefer to proceed with this? Do you feel like forking the repository and contributing? Or do you prefer if I take care of the implementation and keep the conversation over here?

[twoixter]

Sure! I can fork and make a PR. I'll try to send it as soon as I can.