ドッカンソースであそぼう!

この記事は C++ Advent Calendar 2017 の 3日目の記事です。

今回は「ドッカンバトル」の詫びソースコードで遊んで
エンジニアとして何かのアウトプットできればと思います。

最初のお断り

この記事は該当ソースコードから何か読み取れるか
私の私見を交えて何かアウトプットしているだけで
公式が公開したソースコード以外、
バンダイナムコエンターテインメントさんと「ドッカンバトル」に一切関わりがありませんので
この記事の内容を関係各所に問い合わせするのはおやめくださいね
内容もあまりC++と関係ないかもしれませんが、大目に見てください!(C++で書かれてるだけ)

目次

1.読んでみる
2.動かしてみる
3.修正してみる

読んでみる

f:id:kou_yeung:20171118183051p:plain

CardDatas

CardDatas CardModel::getMasterCardDatasByIds(const std::vector<uint32_t>& masterCardIds) const
{
    vector<CardDataPtr> results;
    // ...
    {
        results[i] = _cardDataCache.store(make_shared<CardData>(stmt));
    }
    // ...
    return results;
}

以下の定義があることが読み取れます

class CardData {};
using CardDataPtr = std::shared_ptr<CardData>;
using CardDatas = std::vector<CardDataPtr>;

_cardDataCache

名前の通り、カードデータをキャッシュするクラスです

results[i] = _cardDataCache.fetch(masterCardIds[i]);
results[i] = _cardDataCache.store(make_shared<CardData>(stmt));

メソッドの呼び出しによって

class CardDataCache
{
public:
    CardDataPtr fetch(uint32_t id) const
    { /* ... */ }
    CardDataPtr store(CardDataPtr cardData)
    { /* ... */ }
private:
};

こんな感じなクラスがある

DatabaseManager

string sql = form(/*簡略*/);
DatabaceManager::getInstance()->query( sql, [this, &result, &i]( sqlite3_stmt* stmt ) {
    // ...
});

DatabaceManager は Singleton か ServiceLocator と思います
query( ... ) は クエリを実行した結果をラムダ式を呼び出す

class DatabaseManager
{
public:
    static DatabaseManager* getInstance()
    {
        static DatabaseManager _instance;
        return &_instance;
    }
    void query(const std::string& sql, std::function<void(sqlite3_stmt*)> cb)
    { /* ... */ }
};

こんな感じなクラスがある

SQL

string sql = form("SELECT * FROM cache.cards where id IN (%s);", join(masterCardIds, ",").c_str());

cache.cards テーブルから指定されたID一覧のデータを取ってくる
SELECT 文は ORDER BY を組み合わせて使うのが多いが、
この一文では ORDER BY がなく、もしかしてと調べたら

SQLの仕様上SELECTで取得できる列の並びが保証されているのはORDER BYが指定されている場合のみです。
指定されていない場合の挙動は仕様には定義されていないので、実装依存となります。

なるほど、これは今回不具合の元凶ですね。
今回の件、私が実装担当としても踏まない自信がないわ・・(SQLが詳しくないもので)

実装方法から推測ですが、
戻り値 CardDatas の 並び順は masterCardIds と同じ順で並んでほしいし、
そうでなければ、resize() と indexer の代わりに reserve() と push_back() で実装できるはずです。
それと、masterCardIdsがソート済を要求する場合
このAPIの使い勝手がとても悪くなります。
よって ORDER BY でソートしても正しく動作しない可能性がある。

ただし、

SQLではIN句を作っているので、
ORDER BY FIELD構文が使えるDBなら、
割と安全に返す順番を制御できるSQLが組めます。
SQLiteでは使えないかもしれないです

と教えていただいた 「だらりんさん(@hondarer)」 に感謝します。
Twitter上の長文での付き合い、ありがとうございました。

さて
プログラム全体のイメージができました。
では次に行きましょう!

動かしてみる

クラスが提供されたメソッドが分かったので、
疑似的な振る舞いを実装して動かそう!


一部インタフェースは分かりやすいように
受け取りパラメータなど変更しています、
その都度、コメントで記載します。

CardData

class CardData
{
public:
    // 本来は sqlite3_stmt* を受け取りますが、IDがわかればいいのでID簡易化しました
    // CardData(sqlite3_stmt* stmt) {/* ... */}
    CardData(uint32_t id):_id(id) {}
    uint32_t _id; // カードID
};
using CardDataPtr = std::shared_ptr<CardData>;
using CardDatas = std::vector<CardDataPtr>;

CardDataCache

class CardDataCache
{
public:
    // id を受け取り、キャッシュされた CardDataPtr を返します。なければ nullptr を返す
    CardDataPtr fetch(uint32_t id) const
    {
        auto it = cache.find(id);
        return (it != cache.end()) ? it->second : nullptr;
    }
    // CardDataPtr を受け取り、キャッシュされてない場合キャッシュに登録する
    CardDataPtr store(CardDataPtr cardData)
    {
        auto it = cache.find(cardData->_id);
        if (it == cache.end()) cache.emplace(cardData->_id, cardData);
        return cardData;
    }
private:
    std::map<uint32_t, CardDataPtr> cache; // キャッシュ : KEY( カードID ) VALUE(CardDataPtr)
};

DatabaseManager

class DatabaseManager
{
public:
    static DatabaseManager* getInstance()
    {
        static DatabaseManager _instance;
        return &_instance;
    }
    // 今回の不具合を再現するのが目的です
    // 受け取るID一覧を呼び出しデータの順が保証しなければ再現できるため
    // std::vector<uint32_t> を受け取り、順がランダムのCardDataPtr を渡すように変更しています。
    // void query(const std::string& sql, std::function<void(sqlite3_stmt*)> cb)
    void query(const std::vector<uint32_t>& ids, std::function<void(CardDataPtr)> cb) const
    {
        // ids をコピーしてシャッフルする
        auto result = std::vector<uint32_t>(std::begin(ids), std::end(ids));
        std::random_device seed_gen;
        std::mt19937 engine(seed_gen());
        std::shuffle(std::begin(result), std::end(result), engine);
        // シャッフルしたID一覧で CardDataPtr を生成しラムダ式を呼び出す
        for (const auto& id : result)
        {
            cb(std::make_shared<CardData>( id ));
        }
    }
};

CardModel

class CardModel
{
public:
    CardModel()
    {
        // 公式サイトから引用
        //=======================
        // ・表示に必要なカードIDリスト: 1, 2, 3, 4, 5
        // ・メモリ上のカードIDリスト: 1, 2, "空白", 4, 5
        //=======================
        // とりあえず、[1,2,4,5] のデータをキャッシュさせれば 3 を取得時空白(nullptr)になります
        _cardDataCache.store(std::make_shared<CardData>(1));
        _cardDataCache.store(std::make_shared<CardData>(2));
        _cardDataCache.store(std::make_shared<CardData>(4));
        _cardDataCache.store(std::make_shared<CardData>(5));
    }
    // 実装は公式が提供されたソースコードをベースにしてインターフェースに合わせて調整しています
    CardDatas getMasterCardDataByIds(const std::vector<uint32_t>& masterCardIds) const
    {
        std::vector<CardDataPtr> result;
        result.resize(masterCardIds.size());
        size_t exists = 0;

        for (int i = 0; i < masterCardIds.size(); i++)
        {
            auto p = result[i] = _cardDataCache.fetch(masterCardIds[i]);
            if (p != nullptr) {
                exists++;
            }
        }
        if (masterCardIds.size() == exists) {
            return result;
        }

        // 今回のサンプルでは使用しないため、コメントアウトしました。form(...) join(...) の実装を省く!
        // string sql = form("SELECT * FROM cache.cards where id IN (%s);", join(masterCardIds, ",").c_str());
        int i = 0;
        // 注 : SQL 文字列を受け取る代わりに、直接 ID一覧を受け取るようにしています
        DatabaseManager::getInstance()->query(masterCardIds, [this, &result, &i](CardDataPtr cardData){
            if (result[i] == nullptr) {
                result[i] = _cardDataCache.store(cardData);
            }
            i++;
	});
        return result;
    }
private:
  mutable CardDataCache _cardDataCache;
};

上記コードを実行するために

int main()
{
  auto model = CardModel{};
  auto masterCardIds = std::vector<uint32_t>{ 1,2,3,4,5 };
  auto cardDatas = model.getMasterCardDataByIds(masterCardIds);
  for (const auto& card : cardDatas)
  {
    std::cout << card->_id << " ";
  }
}

wandbox を使って実行してみよう!
[Wandbox]三へ( へ՞ਊ ՞)へ ハッハッ

// 実行結果 : 再現できました。
1 2 4 4 5

動作するソースコードを Gist に乗せました。
※上に解説したままなので、読む気がなければ飛ばして「修正してみる」にスクロールだ!
gist.github.com

修正してみる

公式のソースコートは難しく見えますが、
やってることを文章にすると、

1.id 一覧を渡して、id 一覧の並び順に沿ってカードデータを取得したい
2.キャッシュから検索し取得できたら、result[i] に代入する
3.取得できなかったデータがあれば、ストレージから取得してキャッシュしresult[i]に代入する

複数の処理を混ざってるので読み取る時間がかかってしまった印象でした

では、以下に変更してみよう

1.id 一覧を渡して、id 一覧の並び順に沿ってカードデータを取得したい
2.キャッシュされないID 一覧を作成
3.キャッシュされないものをストレージから取得する
4.キャッシュからカードデータ一覧を作成する

やりたいことは同じですが、スッキリ!
この文章に沿って実装したソースコードです
gist.github.com

wandbox を使って実行してみよう!
[Wandbox]三へ( へ՞ਊ ՞)へ ハッハッ

// 実行結果
1 2 3 4 5

やった!正しい結果が表示されました!

あとがき

「読んでみる」では
SELECT 文や ORDER BY の振る舞いを知りました。
普段フロントエンドメインの私にとって
とても有益の知識でした。

「動かしてみる」では
分業体制で作業する人にとって(ゲーム開発では サーバ・クライアント)
実装スケジュールによって実際に動作するAPIを待てないことが多く

疑似的な振る舞いが実装できることで、
・他人の実装を待たなくても作業が進める
テスト駆動開発のモックオブジェクトを作成にとても役に立ちます。

これができれば「作業が早いね」と褒められたり「プログラミングチョットデキル」アピールできたり
ぜひ習得してほしいスキルです。

「修正してみる」では
やりたいことを明確化し読みやすくなった。
久しぶり C++ 読んだり書いたり楽しかった。
ただこれだけです。

修正版ソースコードをレビューしてくださる方に感謝します。[特に サトシさん( @paosidufygthrj ) ]

後あとがき

ネット上、詫びソースコードを見て、
C++ だから」とか「書き方がよくない」とか叩いてるエンジニアもいるかもしれませんが、

中国では「事後孔明」という諺があり(日本語では「げすの後知恵」というのが今知った)
私は『最初からこんな風に書けばよかった』とかは言わない
他人が書いたソースコードは真摯に向き合えば
何かが得られるかもしれません!

それと、
どうすればソースコードが読みやすく書けるかが知りたい方
リーダブルコード ―より良いコードを書くためのシンプルで実践的なテクニック
をお勧めします。

最後に私のポエムを紹介して終わりにします

ではまたどこかで!