焦ったり急いでコーディングしてると忘れがちで結構やらかしている事が多いミス。JSは自由度が高すぎる言語なので決まりをある程度設けないと滅茶苦茶なコードになってしまいますね。前に実際にやらかしたミスや今後も気をつけないと繰り返しそうな凡ミスの例を書き出してみます。
バグの温床
data, attr, propでのデータ取得(get)/更新(set)
上記事に分かりやすくまとまっています。それぞれどのような特徴があるかを知った上で使い分ければミスしません。
$(this)の中身
関数が入れ子になっていると起こりうる現象。thisの参照先が異なるためにバグの温床になりやすいです。他サイトでも説明されきってる感じですが一応。
// 実際には下記のように書かずに$(".hoge").on('change', ~~~と書きますが分かりやすくするために。
$(".hoge").each(function() {
// ここの$(this)と
$(this).find('input').on('change', function() {
// ここの$(this)は違う
console.log($(this));
});
};
id読み込み時の挙動
id属性はuniqueである必要があるので、idが重複していた場合通常は正しくJSが動作しないはず。
昔のIEではJSの挙動がおかしくなっていましたが、ChromeやFireFoxではなんと中で独自に解釈したのか、ID重複後もエラーを吐かずに動作していました。なぜなんだろう。。
最新Ver(現時点で55.0.2883.87 m (64-bit))のChromeで試しにやってみると、・・・
<body>
<div id="hoge">最初のid</div>
<div id="hoge">2つめのid</div>
<div id="hoge">3つめのid</div>
</body>
に対して
この通り、動くみたいです。最初のidに合致する要素を取ってきています。
しかし異常な挙動であることに代わりはないので、うっかりダブらせてしまったら速攻で削除してやるべし。
true/falseの空判定
!!"false"
これはtrueになります。文字列のfalseもtrueもtrueとして扱われるので、わかりづらいです。文字列でfalseになるのは""のみ。関数や変数の中身で判定する時、文字列が返ってくるとわかっているなら必ず === ""の空判定をするべし。
もう一つ、勘違いしやすい判定として
if []
もしくは
if {}
これはfalseになるかと思えばtrue。if文にundefinedやfalse, nullが入れば当然falseとして扱われますが、[]/{}自体は中身が空にもかかわらず1つの空でないオブジェクトとして扱われるのが原因。配列を判定するとわかっていれば
変数.length > 0
Object.keys(変数).length > 0
とすれば中身が空かどうか判定出来ます。
最後に、曖昧比較(==)と厳密比較(===)。前者は変数の型を半ば無視して判定するので、本来falseなはずがtrueにしちゃう。例えば
100 == "100"
これはCやJavaなどでは明確にfalseになるのですが、JSでは==を使う限りtrue扱いです。StringとNumberで比較して同一とか何の冗談だと。でも企業のコードでこういうの多いです。PHPとかJSみたいな動的型付け言語では変数の型がわかりづらいことで、バグの温床になります。関数に渡す変数の型が違ったせいでエラーを吐いていた場合、この変数の型違いによるものが起こりうるので、余程の理由(ってなんだ？)がない限りは厳密比較を使うべし。
null.~~
例えば要素取得してattributeへアクセスする場合。$("~~")が存在しない場合nullになり、null.attribute名でCannot read propertyとなります。
要素名をtypoによって引き起こすことはテストによって排除出来るので、動的な要素変更があった場合に起こりうるバグです。class名やid属性変更後は特に注意です。
find, closest等による要素取得
viewのツリー構造が予めわかっていればfindだけでなくclosest, next等で要素を狙って取得できますが、ツリー構造に変更があった場合はその取得の仕方も変えないといけません。id属性やclass属性で取得していればclass名、id名を変えない限り影響はないのでまだ安心です(変える時は注意)。
自由度が高いからと言って好き放題やらない
1つのロジックを作るのにも多くの手段がありますが、実際書き方1つ違うだけでシンプルできれいなコードにもクソコードにもなります。
クソコードの例としては
を見れば大体わかりそうなものです。本当にひどい例です（笑）
そうならないためには、言語ごとに役割を明確にもたせておくのが良いかと。
他の酷い例
関数名とreturn内容が合ってなかったり
// タイトルを返すと書いてあるのにコメントを返している
function title() {
～～(処理)
return comment;
};
(残念ながら某所で↑の例を実際に見ました...)
HTMLにif, forなどの複雑な構文を埋め込む(erbとして書きます)
<div class="~~~"> <% if hoge %> <% fuga.each do |f| %> <span class="~~">xxx</span> <% if piyo > book = Book.find ~~ ... <% end %> <% end %> <% end %> </div>
Viewの役割に複雑な構文を突っ込んでいるだけでなく、クエリも走らせています。これでは見づらくて仕方ない。
こういう酷い例を見たら多少リスクを犯してでもついでに直しておいて、次見た時読みやすくしておくのがまともなエンジニアってもんでしょう。リファクタリングで多少のミスが出てもそれが致命的なものでないなら、それは大した傷ではないと思います。寧ろクソコードが新しく生み出され続けることのほうが余程重症なので。
ここではJS、特にjQueryを使った際のバグについて書きましたが、他の言語でも色々ひどい例は見てきたので、また何か酷いのを見つけたら書くと思います（笑）
