exercism / exercism/cpp

Improve error messages in "robot-simulator"

オープン
#526 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
C++
スター
290
フォーク
244
PR マージ指標
30日以内にマージされた PR はありません

説明

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?

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

調査の方向性

robot_simulator_test.cpp と既存の Catch2 テスト設定から始めます。Catch2 の StringMaker と CATCH_REGISTER_ENUM のドキュメントを確認し、提案されている診断サポートを追加して robot-simulator テストを実行します。失敗した比較でプレースホルダーや整数ではなく、ペアの値と Bearing の名前が表示されれば完了です。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
cpp
領域
testing-qa
issue の種類
機能追加
難易度
2/5
見積もり時間
1〜3時間
活発さ
停滞
明瞭さ
明確に書かれている
初心者へのやさしさ
45/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。