Improve error messages in "robot-simulator"
- Dominant language
- C++
- Stars
- 290
- Forks
- 244
- PR merge metrics
- No merged PRs in 30d
Description
The tests in the exercise "robot-simulator" compare the result of the member functions `get_position()` and `get_bearing()` with the `operator==` like this:
```cpp
TEST_CASE("A_robots_is_created_with_a_position_and_a_direction")
{
const Robot r;
const std::pair expected_robot_position{0, 0};
REQUIRE(expected_robot_position == r.get_position());
REQUIRE(Bearing::NORTH == r.get_bearing());
}
```
But Catch2 does not know how to print a `std::pair`, and it prints an `enum` or `enum class` like an integer:
```
-------------------------------------------------------------------------------
A_robots_is_created_with_a_position_and_a_direction
-------------------------------------------------------------------------------
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:13
...............................................................................
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:18: FAILED:
REQUIRE( expected_robot_position == r.get_position() )
with expansion:
{?} == {?}
-------------------------------------------------------------------------------
A_robots_is_created_with_a_position_and_a_direction
-------------------------------------------------------------------------------
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:13
...............................................................................
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:19: FAILED:
REQUIRE( Bearing::NORTH == r.get_bearing() )
with expansion:
0 == 2
```
That's not really helpful.
---
Catch2 has the a `StringMaker` for printing custom classes (see the [documentation](https://github.com/catchorg/Catch2/blob/v2.13.6/docs/tostring.md#catchstringmaker-specialisation)), and it has the macro `CATCH_REGISTER_ENUM()` for better error messages when working with enums (see the [documentation](https://github.com/catchorg/Catch2/blob/v2.13.6/docs/tostring.md#enums)).
By adding a few lines somewhere at the beginning of `robot_simulator_test.cpp`
```cpp
// for better error messages
namespace Catch
{
template
struct StringMaker>
{
static std::string convert(const std::pair& value)
{
std::string result = "std::pair{";
result += StringMaker::convert(value.first);
result += ", ";
result += StringMaker::convert(value.second);
result += '}';
return result;
}
};
}
CATCH_REGISTER_ENUM(robot_simulator::Bearing,
robot_simulator::Bearing::NORTH,
robot_simulator::Bearing::WEST,
robot_simulator::Bearing::SOUTH,
robot_simulator::Bearing::EAST)
```
we would get better error messages:
```
-------------------------------------------------------------------------------
A_robots_is_created_with_a_position_and_a_direction
-------------------------------------------------------------------------------
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:36
...............................................................................
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:41: FAILED:
REQUIRE( expected_robot_position == r.get_position() )
with expansion:
std::pair{0, 0} == std::pair{0, 1}
-------------------------------------------------------------------------------
A_robots_is_created_with_a_position_and_a_direction
-------------------------------------------------------------------------------
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:36
...............................................................................
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:42: FAILED:
REQUIRE( Bearing::NORTH == r.get_bearing() )
with expansion:
NORTH == SOUTH
```
---
AFAIK there are only two possible problems:
1. `robot_simulator_test.cpp` becomes more complex. But IMHO those 22 lines can be ignored.
2. That would effectively enforce the use of `enum` or `enum class` for `Bearing`. IMHO that's not a problem for us because we want idiomatic solutions, and that's `enum` or better `enum class`.
IMHO the benefits outweigh these problems.
What do you folks think?
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.