読者です 読者をやめる 読者になる 読者になる

boost::optional<ErrorCode>を戻り値にする設計はダメかもしれない

C++

以前、「正当性チェック関数でのエラーの返し方」というエントリでboost::optionalのような書き方を推奨したのですが、これはやらない方がいいかもしれません。


以下のようなファイルを開く関数があった場合、

struct ErrorCode {
    enum enum_t {
        FileNotFound
    };
};

boost::optional<ErrorCode::enum_t> FileOpen(const std::string& path);

これを使うユーザーコードで、以下のように書いてしまうことがよくありました:

if (FileOpen("a.txt")) {
    // ファイルが開けた!・・・えっ?
}

「ファイルが開けたかどうか」をチェックして正常だったらthen節に流れることを意図していますが、これは間違いです。FileOpen()関数の戻り値は、boolに変換されてtrueになったらエラーなので、意味が逆なのです。
以下のようにユーザーが書けば、意図が明確な良いコードと言えるでしょうが、前者のコードを禁止することは難しそうです。

if (boost::optional<ErrorCode::enum_t> error = FileOpen("a.txt")) {
    // ファイルが開けなかった!
}

この問題を解決する設計として考えられるのは以下の2つの方法です。

  • エラーを参照の引数として返す

Boost.FilesystemやBoost.Asioでとられている設計です。
エラーコードの型と変数名を書くことをユーザーに強制できます。if (error)のthen節を「ファイルが開けた」という意味で使うユーザーはいないでしょう。

boost::optional<ErrorCode::enum_t> error;
FileOpen("a.txt", error);

if (error) {
    // ファイルが開けなかった!
}
  • エラーコード用の型を用意し、boolへの変換を提供しない

boolへの変換は逆の意味で使えてしまうので、代わりにis_error()のような関数を提供する方法もありえます(関数名はもうちょっと考える必要があります)。if文内でエラーコードの変数を定義することができなくなりますが、ユーザーが使い方を間違えるよりはいくらかマシだと思います。

error<ErrorCode::enum_t> error = FileOpen("a.txt");
if (error.is_error()) {
    // ファイルが開けなかった!
}


boost::optionalを戻り値とするときに変数の定義を強制できればそれが最もスマートだと思うのですが、果たしてそんなことはできるでしょうか。(テンプレートメタプログラマへの呼びかけ)
Loki::CheckReturnも妥協してた気がする。