LoginSignup
2
1

More than 3 years have passed since last update.

読みやすいコードのためには lodash の isEmpty や size を使わない方がよいかも。というかよい。

Posted at

lodash はとてもいい

とてもいいです。lodash。
ただ、いくつかの点で使い方を誤ると、いまいちなコードになります。

特に、isEmpty と size については前から気になっていたので書いておきます。

最近、こちらの記事をよみました

lodashの真偽値判定まとめ - Qiita
https://qiita.com/ko-flavor/items/83e06afde5508541edaa

isEmptyの挙動についてしっかり書かれています。

lodashの size関数でも似たようなものになります。

_.size(value) === 0

となるものを、isEmpty(value) = true としているようです。

isEmptyもsizeも引数として、配列やオブジェクトや文字列、なんでも受け付けるはずです。

細かくは知りません。調べていません。使ってはだめなのがわかるので使わないものなので、詳しく知らべる気しないです。

なぜだめなのか

読みづらいコードになってしまうからです。

コード上で、こういうコードをみかけたとします。

// [A]
  if (_.isEmpty(value)) {
    funcA(value);
  }

これと、下記は似たようなものに見えるかもしれません。

// [B]
  if (value.length === 0) {
    funcA(value);
  }

ですが、優れているのは[B]のコードです。

なぜか。

[A]のコードは、valueに何がはいっているか、さっぱりわかりません。
[B]のコードは、valueに、stringかarrayかが入っていることが即時に読み取れます。

stringかarray以外なら、基本、動かないので[B]の処理が書かれていればvalueの中身を類推できます。
[A]のコードでは、無理。

自分は、lodashのいくつかの点で満足がいかないので
Parts.js というライブラリを作っています。lodashみたいな関数群をそろえているのですが、互換性はないです。
https://github.com/standard-software

ですので、lodashのisEmptyを作りたく気持ちも分からなくはないですし、作ったら作ったでちょっと使えるかも、と思わなくもないところですが、やはり isEmpty は作るべきではないです。

Parts.js では、parts.propertyCount というオブジェクトのプロパティ個数をカウントする関数を作っています。
オブジェクトが渡されなければエラーを吐いて落ちる関数です。

// [C]
  if (parts.propertyCount(value) === 0) {
    funcA(value);
  }

これも、value内部がオブジェクトの場合では、[A]のコードと同じように動きます。
ですが、[A]のコードより[C]のコードは確実に優れています。

なぜか。
先程と同じ理由です。

[A]のコードは、valueに何がはいっているか、さっぱりわかりません。読み取れない。
[C]のコードは、valueに、objectが入っていることが即時に読み取れます。

これがコードリードに大きく影響します。

混乱したスパゲティコードを素早く解きほぐすために

自分は多くのプロジェクトに参加してきました。
ぐちゃぐちゃなコードをよく見てきました。
そして、それを素早く読み取り、リファクタリングしていきます。

そういうときに[A]系のコードがガシガシ書かれている何百行、何千行のコードを見ると、
ここから読み取れることが少なすぎて、実際に動かしてみて、console.log 仕込まないと、どういう動きになるのかよくわからなくなるのです。

ほとんどのプロジェクトが大抵、混乱したコードに悩まされると思います。
[A]のコードが大量にかかれていて、
valueに、0か、空文字か、空配列か、空オブジェクトか、
なにが入ってくるかわからないようなぐちゃぐちゃなコードの中で
それを読み解こうとして必死になって時間を食います。

たった1行のことですが、
[B]や[C]のようなコードで書いてくれる癖をつけておいてくれたら
こんなにスパゲティにもならないだろうと思うわけです。
そうするとコードリードが実にやりやすいのです。

もっと上手に

たとえば、[B]だと配列か文字列かとわかりますが、もっとよく書くことができます。

// [D]
  if (value === '') {
    funcA(value);
  }

これなら、valueが文字列確定だから、とてもよいです。[B]より優れています。

また、次のような数行関数を作ればいいでしょう。

// [E]
const arrayCount = (value) => {
  if (!Array.isArray(value)) {
    throw new Error('arrayCount');
  }
  return value.length;
}
:

  if (arrayCount(value) === 0) {
    funcA(value)
  }

これなら、valueは配列に確定するので、これも[B]より優れています。

繰り返しになりますが、
プロジェクト全体を通して[A]のコードで書かれていると、プログラム全体は何がなにやらわからなくなりがちです。

プロジェクト全体が[C][D][E]のコードで書かれていれば、丁寧で読みやすいコードになります。

[A]よりも[B]、さらには、[C][D][E]がよい、と判断できる人が書いたプログラムはとても読みやすくなります。

動かしてconsole.logの動作確認などしなくても、コードを読めば、そこに書かれている動きがすぐにわかるのです。

というわけで、

lodashの _.isEmptyや _.size は書かれているコードを読んでも何を判断しているのかが即座にはわからなく、読みにくいコードになってしまい、また代替手段も多くあるために、使うべきではない関数になってしまっています。

ご参考にしていただけると幸いです。

2
1
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
2
1