qsonaのブログ

プログラマーです。

LT「チーム開発においてNode未経験者の学習コストを下げるための工夫」の補足

先日、東京Node学園18時限目「Node.js 4.0の話」の回で、LTをさせていただきました。

Node学園 18時限目 Node.js v4.0の話 - connpass

資料はこちらです。

speakerdeck.com

たかが10分といえど、120人*10分=20人時間というプレッシャー。勉強会等で発表するのはこれが初めてで、この1週間はひたすらこのLTのことだけを考えて生活していたような。
テーマは初めに思い切って設定したものの、まずはこの1年の開発を振り返り何がポイントだったのかを考えるところから・・・、
そこからプロットを作って話す練習をしてみると、どうも話が繋っていなかったり、技術的な話が少なすぎて、これじゃNodeの会合に来ているお客さんに申し訳ないよなぁ、というような内容だったりと、内容をまとめ上げるのに苦心しました。

そんな苦労もあった準備の甲斐もあり、良い反応も頂き、ひとまずホッとしている次第です。

会場にいらした皆様、ご清聴ありがとうございました。

さて、資料には一応載せたけれど、発表時にもスライド上でも十分説明できていない点について、このエントリで少し補足をさせていただきたいと思います。

lodashのバージョンについて

lodashはv2.4.1でしばらくの間リリースが止まっていましたが、約1年前くらいにv3がリリース、そしてそろそろv4がリリースされそうです。

特徴として、v2 => v3, v3 => v4 でのbreakingな変更が割と多いです。運用に入ってしまうとメジャーバージョンを挙げるのは結構大変になってしまうイメージです。
資料にちらっと書きましたが、いまから開発始めるなら、v4が出るまで(出てからしばらくまで)、依存先をedge(github上のmasterの最新)にしておくのが良いんじゃないかなと思っています。

v4の特徴としては、今まで1つの関数にいくつかの引数の渡し方ができていたところを、関数自体を分けるように変更がなされます。
例えば_.uniqという関数は、コレクションを渡すと重複する値を1つだけにした配列を返しますが、この_.uniqの引数に関数を与えると、その関数を通した結果に対して、uniqをかける、という仕様でした。
これが、_.uniqと_.uniqByという2つの関数にわかれ、関数を渡したければ後者を使うことになります。

このように、シンプルな方向に倒していくのがlodashの良い所の一つであります。

その他、aliasを大量に削除するようです。underscoreにあったメソッドに対して、lodash v3で別の名前をつけておいて、元の名前にはaliasを貼っていたが、v4でそのaliasを削除するなんてケースも。。そこまでやるか・・？？(lodashの作者jdaltonとunderscoreの作者jashkenasは仲悪いです)

なお、バグはそれなりにあります。中規模くらいでガッツリ使っていると出くわすこともあると思います。特にv3の初期は、遅延評価まわりのバグが結構ありました。issueを軽く追っておいたほうが良いです。

async#waterfallとneo-async#angelFallの話

ここらへんは、書き方オタクとしては結構こだわりのあるところで、キチンと書いておきたい話なのですがまた今度の機会にします><

lodashのクエリの書き方

オブジェクトが並ぶ配列arr中から、プロパティxの値が2であるものを見つけるとき。 _.find(arr, function(obj) { return obj.x === 2; })のがunderscoreからある書き方ですが、これを
_.find(arr, { x: 2 })という書き方ができます。さらに別の書き方として、
_.find(arr, 'x', 2)という書き方もあります。面白いですね。

個人的には2つめの_.find(arr, { x: 2 })が良いかなと思っています。
3つめにしない理由は、例えば「xが2, yが3のものを探す」ときに_.find(arr, {x: 2, y: 3})という書き方ができますが、3番目の書き方だと応用が効かなくなってしまうからです。

余談ですが、lodashあるあるで、_.includesを使うべき場面で間違って_.someを使ってしまう、というのがあります。
前者は単なる厳密な比較で含まれていればtrueを返す、後者は上の_.findと同じ感じで、見つかったらtrueを返す。

あるオブジェクトそのものがある配列に入っているかを調べたく、つまり_.includes(arr, obj)としたいところで、
間違えて_.some(arr, obj)なんて書くと、オブジェクト同士のプロパティを1つずつ(しかもdeepに)比較しはじめてしまう。
大抵の場合で結果が一致してしまうのでテスト書いてもバグに気づきにくい。

結果、思わぬところでパフォーマンスの問題を引き起こしてしまうかもしれませんね。
何度も見かけた事例ですが、lodashオタクならコードレビューで間違いに気付くことが出来ますw

undefinedチェックの話

LTで最も反応が良かった、undefinedチェックの話です。

資料中では以下の4点の方法を挙げています。

  1. x === undefined
  2. x === void 0
  3. typeof x === 'undefined'
  4. _.isUndefined(x)

ちなみにLT中で会場の方にどれを使うか挙手でアンケートをとらせていただきましたが、大体挙手の比率は 6:4:10:1 くらいだった印象です。

さて、自分はtypeof x === 'undefined'が良くないと考えている旨を話しました。その点を整理したいと思います。

まず、typeof x === 'undefined'は1つのJavaScriptのイディオムです。この書き方をすると、もし仮にxがそもそも変数として定義されていない場合でも、ReferenceErrorを吐くことがなく、上の式の結果はtrueになります。ブラウザ上で、特にグローバルに変数xが宣言されているかどうかをチェックする場合などに有用であることから、よく使われます。ReferenceErrorを吐かないから安全、という言われ方をすることもあります。

しかしながら、Node.jsではグローバル変数をチェックすることはまずありません。そのときに、「ReferenceErrorを吐かない」ことが「安全」なのか？という疑問が生じます。

例えば変数名をtypoしてしまった。そのときはReferenceErrorを吐くのがむしろ正しい動作で、そうならないということは、吐かれるべきエラーを握りつぶしていることに他ならないわけで、それによりミスに気付く機会を逸するかもしれません。"良くない書き方"であると考えるのはその点です。

さて、書き方をどれにするかでいうと、少し抵抗ある方もいるかもしれませんが、今時は x === undefined で問題ないと言ってよいでしょう。
一応var undefined = 1;されることもあるよ(だけどそんなことする人居ないから気にしないよ)と話しましたが、
Node 4.xで'use strong;'するとそれもできなくなるそうです。

というわけで、一番シンプルなx === undefinedで良いんじゃないでしょうか。

書き方オタクが複数人いたら？

これに対する、自分の解決策は以下です。

  1. 最終決定をする人(リーダー)は1人にする。
  2. 「書き方が統一されている」ことが最重要であり、 全ての書き方オタクはそれを肝に銘じている。

1の方は、書き方に限った話ではないでしょう。とにかく決定する人が2人いると面倒この上ありません。成功事例とした開発も、初期にダブルリーダーのような状態になったときは良くなかった。

2の方は、私のLTで極意の3つめとして話しているものです。これは他をさしおいてなによりも重要だと私は考えています。

良い実例があります。
僕がいっしょに仕事した中での代表的な書き方オタクに、neo-asyncという人物(?)がいます。ちょうど僕の前にLTを行った人です。

neo-async

suguru03/neo-async · GitHub

LT 「Neo-Async」

suguru03.github.io

僕と彼はしょっちゅう書き方について議論をします。その結果、50%は最終的に意見が合致しますが、意見が合わずに終わることも50%あります。

例えば、この記事の上のほうでlodashのクエリの話をしていますが、彼は3つめの_.find(arr, 'x', 2)という書き方が良いと言います。彼は何か話すときは常にベンチマークを取っており、確かこの件ではそれのほうが2倍くらい速く、わざわざ速い書き方があるのでそれを放棄する必要もなかろう、という主張でした。

私はというと、どうせ2つめの_.find(arr, { x: 2 })の書き方は覚えなければいけないので、(_.find(arr, { x: 2, y: 1 })のパターンがあるから)、学習コストを減らしたいので2つめで統一したい。

どちらが正しいということはないと思います。

それ以外にも本当下らないことでしょっちゅう言い合っています。_.eachか_.forEachかなど、自分の会話でなければ即座に「わりとどうでもいい」のAAを貼ってやりたいくらいです。

以前同じチームで仕事したときはたまたま、私がもともといるチームに彼が助っ人的に入ってくれていたこともあり、結果的には最終的には私の意見をほぼ100%通す形になってしまいました。もちろん彼の中では、自分が良いと思う書き方が出来ない不満はあったのではないかと思います。コードレビューでは、"殺してでも うばいとる"ではなく"ゆずってくれ たのむ！"にしたいゆえんですね(LT資料の終わりの方を参照されたし)。

もちろん意見は言ってくれますが、これ以上はその人の考え方あるいは気分だろうな、というところで、それに合わせて修正してくれていました。それはつまるところ、合わせるのが最も大事だと考えているからです。逆に彼のチームに私が入る形だったら、私のほうが合わせることを意識したと思います。(彼はカナダに英語勉強の旅に出てしまうので、少なくとも当分一緒に仕事する機会がないのは残念ですが。)

あとは、あまりロジカルでない主張をむりやり通すと良くなくて(そういうことも実際あった)、個人的にはわりと細かいことでもちゃんと話すのが良いのかなと思っています。ちょっと大変ですが、書き方がバラバラになるというリスクはそれほど大きいということです。

async書き方比較のソースコード

asyncの書き方の例を3つ挙げて比較するスライドを入れましたが、文字の情報にするためにこちらに載せておこうと思います。

準備

var async = require('neo-async');
var getFoo = function(callback) {
  return callback(null, { x: 1 });
};
var getBar = function(x, callback) {
  return callback(null, x + 1);
};
var doBaz = function(callback) {
  return callback(null, 'piyo');
};
var doQux = function(foo, bar, callback) {
  return callback(null, 'baz');
};

plain (callback hell)

var ex_plain = function(callback) {
  // fooを取得する.
  getFoo(function(err, foo) {
    if (err) {
      return callback(err);
    }
    // fooをごにょごにょする.
    foo.hoge = 1;
    foo.fuga = 2;
    // barを取得する.
    getBar(foo.x, function(err, bar) {
      if (err) {
        return callback(err);
      }
      doBaz(function(err, result) {
        if (err) {
          return callback(err);
        }
        foo.piyo = result;
        doQux(foo, bar, function(err, baz) {
          if (err) {
            return callback(err);
          }
          // ...
          callback(null, { foo: foo, bar: bar, baz: baz});
        });
      });
    });
  });
};

async.series (弊社で多い)

var ex_series = function(callback) {
  var foo, bar, baz;
  async.series([
    function(next) {
      // fooを取得する.
      getFoo(function(err, _foo) {
        if (err) {
          return next(err);
        }
        foo = _foo;
        // fooをごにょごにょする.
        foo.hoge = 1;
        foo.fuga = 2;
        next();
      });
    },
    function(next) {
      // barを取得する.
      getBar(foo.x, function(err, _bar) {
        if (err) {
          return next(err);
        }
        bar = _bar;
        next();
      });
    },
    function(next) {
      doBaz(function(err, result) {
        if (err) {
          return next(err);
        }
        foo.piyo = result;
        next();
      });
    },
    function(next) {
      doQux(foo, bar, function(err, _baz) {
        if (err) {
          return callback(err);
        }
        // ...
        baz = _baz;
        next();
      });
    },
  ], function(err) {
    if (err) {
      return callback(err);
    }
    callback(null, { foo: foo, bar: bar, baz: baz });
  });
};

async.waterfall 1 (推奨)

var ex_waterfall = function(callback) {
  var foo, bar;
  async.waterfall([
    function(next) {
      // fooを取得する.
      getFoo(next);
    },
    function(_foo, next) {
      foo = _foo;
      // fooをごにょごにょする.
      foo.hoge = 1;
      foo.fuga = 2;
      // barを取得する.
      getBar(foo.x, next);
    },
    function(_bar, next) {
      bar = _bar;
      doBaz(next);
    },
    function(result, next) {
      foo.piyo = result;
      doQux(foo, bar, next);
    },
    function(baz, next) {
      // ...
      next(null, { foo: foo, bar: bar, baz: baz });
    }
  ], callback);
};

async.waterfall 2

var ex_waterfall2 = function(callback) {
  async.waterfall([
    function(next) {
      // fooを取得する.
      getFoo(next);
    },
    function(foo, next) {
      // fooをごにょごにょする.
      foo.hoge = 1;
      foo.fuga = 2;
      // barを取得する.
      getBar(foo.x, function(err, bar) {
        next(err, foo, bar);
      });
    },
    function(foo, bar, next) {
      doBaz(function(err, piyo) {
        next(err, foo, bar, piyo);
      });
    },
    function(foo, bar, piyo, next) {
      foo.piyo = piyo;
      doQux(foo, bar, function(err, baz) {
        next(err, foo, bar, baz);
      });
    },
    function(foo, bar, baz, next) {
      // ...
      next(null, { foo: foo, bar: bar, baz: baz });
    }
  ], callback);
};