exercism / exercism/cpp

Improve error messages in "robot-simulator"

Đang mở
#526 1 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
C++
Star
290
Fork
244
Chỉ số merge pull request
Không có pull request nào được merge trong 30 ngày

Mô tả

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?

Hướng dẫn đóng góp

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.