spine/Json.hを使うべきでない4つの理由

2014-03-15 21:43:00

Cocos2d-x で JSON フォーマットを扱うためのライブラリを探すと、大抵最初に見つかるのは spine/Json.h です。

Cocos2d-x に組み込まれているため、何かのライブラリを追加で入れる必要はありません。 使っている人も多いため、ちょっと検索すれば使い方も分かります。

そんな便利な spine/Json.h。 しかし、spine/Json.h は決して使うべきではありません。 その理由は、以下の4つです。

  • リンクリストかつ線形探索なのでクソ遅い
  • 型の間違いを無視する
  • インターフェースが変わりやすい
  • キーの大文字小文字を無視する

この4つについて詳細に説明します。

なお、ここではコミットID 625b9501f320a08e6d3aff2ee2ad4e04c6872cc0 時点の spine/Json.hspine/Json.cpp について書いています。

リンクリストかつ線形探索なのでクソ遅い

この辺を見れば分かりますが、オブジェクトから要素を取ってくるために、リンクリストを線形探索しています。

これは配列の探索も同様です。n 番目の要素を取ってくるために、リンクリストを先頭から順番に辿っていく実装になっています。

これだけでもう、使うべきでないというのは確定です。 もちろん線形探索で問題のないケースというのも存在します。 しかし似たような選択肢が大量にある中、わざわざ遅い実装を使う理由なんてほぼありません。

なお、Cocos2d-x内には、spine/Json.h以外にも、最近ならrapidjson、古いのならJsonCppが入っているようです。 追加でライブラリを入れるのが面倒なのであれば、これらを利用するのがいいでしょう。

型の間違いを無視する

この辺を見れば分かります。 Json_getItemJson型の値を取り出し、それを整数として解釈しています。

これ自体は普通の処理です。 問題は、型に対するエラー処理が何も無いことです。

例えば、取り出した値の型が文字列だった場合には、何のエラーも出さず、単に 0 を返します。 null の場合も同様、何のエラーも出さずに、単に 0 を返します。 しかもこれは、引数のdefaultValueとは無関係に 0 を返します。 defaultValueを返すのは、オブジェクト型のデータからnameの要素が見つからなかった場合だけです。

Json_getStringの場合も同様に、エラー処理が何もありません。 値が文字列以外だった場合、nullptrが返されます。

Segmentation Faultで落ちないだけマシだと言えますが、最低でもassertぐらいは入れておくべきです。 可能なら例外を投げるなどの処理を入れた方がいいでしょう。

なお、spine/Json.hで型をちゃんと見た上で値を取り出すなら、以下のようになります。

Json* p;
if (p->type != Json_Object)
    throw "オブジェクト型じゃない";
Json* q = Json_getItem(p, "hoge");
if (q == nullptr)
    throw "hogeが見つからなかった";
if (q->type != Json_Number)
    throw "hogeの値が数値型じゃない";

// 無事目的の値を取得
int result = q->valueInt;

インターフェースが変わりやすい

以前はJson_getItemAtJson_getSizeという関数がありましたが、今はもう消されています

つまり配列から要素を取得したり、サイズを取得するには、このような実装を書いてあげる必要があるということです。 また、Json_getItemAtを使っていた人は、新しいCocos2d-xのバージョンに乗り換えた際に、この部分を修正する必要があります。

spine/Json.hはSpineのためのJSONライブラリなので、Spine側で使わない関数は消されるし、もしかしたらspine/Json.hが丸ごと消える可能性があります。 そのようなライブラリを使い続けるというのは、後のコストを考えるとかなり危険だと言えます。

キーの大文字小文字を無視する

比較の処理を見れば分かるように、tolower小文字に変換した上でキーを比較しています。

tolowerの第2引数を省略した場合、グローバルなロケールが使われます。 そして、ロケールによってtolowerの返す値は異なります。 ロケールによって結果が異なる例はcppreference.comのtolowerなどを見るといいでしょう。

このようなグローバルな値に依存するということはつまり、最初の検索で成功したのに、次の検索では失敗する可能性があるということです。 どこかのライブラリがふとした拍子にstd::setlocaleを呼び出しただけで、意図しない結果になることがあるのです。

このような比較をしているspine/Json.hは使うべきではないでしょう。

最後に

spine/Json.h を使うべきではない理由を4つ説明しました。 しかし実際のところ、これは spine/Json.h が悪いわけではありません。

そもそも spine/Json.h は、Spine で吐き出された JSON を読むために作られたライブラリです。 その JSON は、要素数が少ないため線形探索でも問題ありません。 自動で生成されたものを Spine の内部で読むだけだから、Spine を使うユーザにとっては型の間違いなんて関係ありません。 同様の理由で、大文字小文字を無視されても問題ありません。 配列のデータを取ってくる処理も必要ありません。

つまり spine/Json.h は、目的に対して十分な機能を有しています。 しかし、決して汎用的なJSONライブラリではありません。

spine/Json.hは利用せず、他のJSONライブラリを使うようにしましょう。

Cocos2d-x の CREATE_FUNC をマシな実装にした

2014-01-28 14:13:00

Cocos2d-x には CREATE_FUNC というマクロがあります。これは以下の実装になっています。

#define CREATE_FUNC(__TYPE__) \
static __TYPE__* create() \
{ \
    __TYPE__ *pRet = new __TYPE__(); \
    if (pRet && pRet->init()) \
    { \
        pRet->autorelease(); \
        return pRet; \
    } \
    else \
    { \
        delete pRet; \
        pRet = NULL; \
        return NULL; \
    } \
}

これを以下の様に使うことで、autorelease 済みの TestLayer オブジェクトを生成できます。

class TestLayer : public cocos2d::Layer {
public:
    bool init();
    CREATE_FUNC(TestLayer);
};

ただ、これには問題があります。 マクロを見れば分かるように、createinit 関数はパラメータを持ちません。 つまり、初期化する際に引数を渡したい場合、CREATE_FUNC を利用できません。

もし値を渡したい場合、CREATE_FUNC マクロがやっていることを手動で書く必要があります。

class TestLayer : public cocos2d::Layer {
public:
    bool init(int n, std::string s);
    static TestLayer* create(int n, std::string s) {
        auto p = new TestLayer();
        if (p->init(n, s)) {
            p->autorelease();
            return p;
        } else {
            delete p;
            return nullptr;
        }
    }
};

これはちょっと書くのが面倒過ぎます。 さらに init 関数のオーバーロードが増えると、create 関数の実装も増え、更に面倒になります。

これを解決するため、以下のクラスを作りました。

#include<utility>

template<class Derived>
struct create_func {
    template<class... Args>
    static Derived* create(Args&&... args) {
        auto p = new Derived();
        if (p->init(std::forward<Args>(args)...)) {
            p->autorelease();
            return p;
        } else {
            delete p;
            return nullptr;
        }
    }
};

Variadic Templates や Perfect Forwarding を使っているので、C++11 限定です。Cocos2d-x 3.0 を使うなら問題にならないでしょう。

これは以下の様に利用します。

class TestLayer : public cocos2d::Layer, create_func<TestLayer> {
public:
    bool init(int n);
    bool init(int n, std::string s);
    using create_func::create;
};
auto p = TestLayer::create(10); // init(10) が呼ばれる
auto p2 = TestLayer::create(10, "aaa"); // init(10, "aaa") が呼ばれる

CRTP を使っているため、create_func<TestLayer> と書く手間が発生します。が、それを書くだけで create 関数に値を渡すことでき、init 関数のオーバーロードもできます。

create 関数を書くのが面倒になったら、このクラスを利用することを考えましょう。

Cocos2d-x 3.0 beta の Vector がリークする

2014-01-26 11:21:00

Cocos2d-x 3.0 beta では CCArray に取って代わる Vector というクラスが作られました。

便利になって良いと思うのですが、不満な点が2つほどあります。

  • operator[] が定義されていない
  • iterator 経由でアクセスするとリークする

どちらも根は同じ問題です。つまり、Object を生のポインタのまま格納していることことが問題なのです。

現在の最新版のソースでは、以下のようになっています。

template<class T>
class Vector {
...
private:
  std::vector<T> _data;
};
Vector<Sprite*> v;
v.pushBack(Sprite::create(...));

Sprite は、Object を継承したクラスなので、メンバなどで保持する際には retain, release が必須です。

Vector クラスは pushBack する際に retain したり、デストラクタで release を呼び出したりと、結構頑張って参照カウントを狂わせないようにしています。

しかし std::vector<Sprite*> という形でメンバに持っている以上、限界はあります。

まず、以下のようなコードが書けません。

template<class T>
class Vector {
public:
  T& operator[](size_t n) { return _data[n]; }
private:
  std::vector<T> _data;
};

これは、以下のようなコードを書くと簡単にリークしてしまうからです。

Vector<Sprite*> v;
v.pushBack(Sprite::create(...));
v[0] = Sprite::create(...); // v[0] の要素を release していないのでリークする
                            // おまけに代入したオブジェクトを retain していないのでアクセス違反を起こす

もう一点、iterator に対して代入ができません。

Vector<Sprite*> v;
...
v.erase(std::remove_if(v.begin(), v.end(), []() { ... }),
        v.end());

Vector が提供している iterator は vector::iterator の typedef であるため、retain, release を正しく呼び出さずにリーク、あるいはアクセス違反を起こします。

これらの問題を解決する一番簡単な方法は、intrusive_ptr のようなクラスを使う(あるいは自作する)ことです。

template<class T>
class Vector {
...
private:
  std::vector<intrusive_ptr<T>> _data;
};

これだけで上記の問題は解決します。operator[] では intrusive_ptr<T>& を返すため、代入してもリークしないし、iterator の各要素は intrusive_ptr<T> 型になるので、代入してもリークしません。

デメリットとしては、ユーザが intrusive_ptr に対する知識を知らないといけないということです。 しかし、intrusive_ptr を使わないというのは、そのデメリットを上回るだけのデメリットがあると考えられます。 手動での参照カウントのミスによってバグを埋め込み、修正に時間を掛けるよりは、最初に intrusive_ptr について調べておいて安全に使ったほうが、トータルに掛ける時間は少なくなると考えられます。 また、Objective-C が ARC (Auto Reference Counting) を導入し、参照カウントの手間を無くすという方向に進んでいる中で、手動参照カウントなんていう時代遅れなことをするのは、ちょっと最新のライブラリとしては残念過ぎるのではないかと思います。

正直、なぜ Cocos2d-x が intrusive_ptr を使わないのかが不思議でなりません。これだけ利用者がいるなら、きっと誰か提案しているはずで、何か理由があって却下されたと思うのですが、コミュニティとかを探すだけの気力も無いので、大人しくオレオレ intrusive_ptr を自作して利用するのでした。