exercism / exercism/cpp

Improve error messages in "robot-simulator"

Offen
#526 1 Kommentar 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
C++
Sterne
290
Forks
244
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Beschreibung

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?

Beitragsleitfaden

Für dieses Repository ist kein Beitragsleitfaden indexiert

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.