Improve error messages in "triangle"
The tests in the exercise "triangle" compare the result of the function with the operator== like this:
TEST_CASE("equilateral_triangles_have_equal_sides")
{
REQUIRE(triangle::flavor::equilateral == triangle::kind(2, 2, 2));
}
By default Catch2 prints an enum or enum class like an integer:
-------------------------------------------------------------------------------
equilateral_triangles_have_equal_sides
-------------------------------------------------------------------------------
/home/user/exercism/cpp/triangle/triangle_test.cpp:9
...............................................................................
/home/user/exercism/cpp/triangle/triangle_test.cpp:11: FAILED:
REQUIRE( triangle::flavor::equilateral == triangle::kind(2, 2, 2) )
with expansion:
0 == 1
That 0 == 1 is neither expressive nor helpful.
Also, I've mentored many solutions that used something other than an enum, e.g.:
namespace flavor {
const int equilateral = 0;
const int isosceles = 1;
const int scalene = 2;
}
int kind(double side1, double side2, double side3);
// or
class flavor {
public:
static const std::string isosceles;
static const std::string equilateral;
static const std::string scalene;
};
std::string kind(double a, double b, double c);
Catch2 has the macro CATCH_REGISTER_ENUM() for better error messages when working with enums (see the documentation).
By adding a few lines somewhere at the beginning of triangle_test.cpp:
// for better error messages
CATCH_REGISTER_ENUM(triangle::flavor,
triangle::flavor::equilateral,
triangle::flavor::isosceles,
triangle::flavor::scalene)
we would get better error messages:
-------------------------------------------------------------------------------
equilateral_triangles_have_equal_sides
-------------------------------------------------------------------------------
/home/user/exercism/cpp/triangle/triangle_test.cpp:15
...............................................................................
/home/user/exercism/cpp/triangle/triangle_test.cpp:17: FAILED:
REQUIRE( triangle::flavor::equilateral == triangle::kind(2, 2, 2) )
with expansion:
equilateral == isosceles
and that would guide students towards using enum or enum class.
AFAIK there are only three possible problems:
-
When the students read the tests they will be guided towards an
enumorenum class, they are less likely to "discover" enums on their own when searching for the best solution. But I'd rather view that "guidance" as positive. -
That would effectively force the use of
enumorenum class, any of those alternative approaches (see above) would lead to a compilation error. IMHO that's not a problem for us because we want idiomatic solutions and that'senumor betterenum class. -
If the students want to add a fourth
degenerateenumerator (for the bonus part in "Dig Deeper"), they will have to add that enumerator in the tests, too, or they wil get this "unexpected enum value":------------------------------------------------------------------------------- equilateral_triangles_have_equal_sides ------------------------------------------------------------------------------- /home/user/exercism/cpp/triangle/triangle_test.cpp:15 ............................................................................... /home/user/exercism/cpp/triangle/triangle_test.cpp:17: FAILED: REQUIRE( triangle::flavor::equilateral == triangle::kind(2, 2, 2) ) with expansion: equilateral == {** unexpected enum value **}
IMHO the benefits outweigh these problems. What do you folks think?
I think the point with the enum/enum class is a valid option. Especially if the solution already tend to point to an enum/enum class. So why not push the users in the right direction with that?
I think this is a very helpful change. I made a PR: https://github.com/exercism/cpp/pull/726