#はじめに
ここは後輩に指導？助言？したコードレビューをつらつらと書いていたりしてます。
今回もjavaScript編と言いつつ殆ど一般論。つーか最近フロントばっかりいじってるなぁ。
実際に現場で見てしまったコードを直していますが、もっといい実例とか修正先あったら教えてください
（流石に生コードを書くのは気が引けるので変数名とかは変えてますけど）。
##そのif文要るの？編
以前も書いたけど、また出てきたので…
###しょにょ６
returnParam = new Array();
var alice,bob,（中略）,zoe; // 実際は一行ずつ定義されてた
$("tr[id='record']").each(function(){
if($(this).find("#alice").is(':checked')){
alice = "1";
}else{
alice = ”0”;
}
（中略）
if($(this).find("#zoe").is(':checked')){
zoe= "1";
}else{
zoe= "0";
}
returnParam.push{
rowNo:$(this).find("#rowNo").val(),
alice:alice,
（中略）
zoe:zoe
}
});
いつものアリスちゃん激おこ案件。おしえてくれごひ、俺はあと何回糞コードを殺せばいい…？
###ランボー怒りの突っ込みどころ
- せめて宣言時に０入れとけばelse節要らないよね？
- そもそもこいつら使い捨てなんだから、変数に入れる必然性皆無だと思うんだけど？
- 一方で「
$(this)」で何回掴めばいいねん！使いまわせるものこそむしろ変数に入れようよ - （htmlレベルで見ると）idかぶってるよねコレ？
- identifierって単語を辞書で引け！
…総じて「どうしてこうなった」。
returnParam = new Array();
$("tr[id='record']").each(function(){ // idはとりあえず放置 以下同様
var _own = $(this); // ここは人によってちょっと違うだろうけど
returnParam.push{
rowNo:_own.find("#rowNo").val(),
alice:_own.find("#alice").is(':checked') ? "1" : "0" ,
（中略）
zoe:_own.find("#zoe").is(':checked') ? "1" : "0"
}
});
if文と変数定義がまるっと消えるので、一要素に付き６行も削れてしまうのでした。
実際あの修正前の冗長な書き方が長すぎるとは思わないものなのだろうか。僕は３回もループすれば確実に思う。
idについては、html生成まで手を出すのが（担当外だし）めんどくさかったので、とりあえず「どうせイテレータとかループ処理で作ってるんだろうし、rowNoとか末尾に付けて、セレクタを$("[id*='record']")にするのがたぶん手っ取り早いので担当者に言っといて」とは言っておいたけどさてどうなることやら（なげやり）。無いとは思うが、まさか手で一行一行書いてないよな…？
###ちなみに
条件演算子で思い出した。むかしのやつ（後輩にレビューで突っ込みを入れた話 JavaScript編）
if (!muted){
volume = Math.max((volume - 0.05),0);
$("#volume").slider('value',volume);
}
これに?:演算子使うと
if (!muted){
volume = (volume - 0.05 > 0) ? volume - 0.05 : 0;
$("#volume").slider('value',volume);
}
になることを思い出したがコレどうなんだろ。僕はどっちでもわかるが、記述で２回計算している?使用パターンに比べればMath.maxのほうが綺麗。
###しょにょ７
またもやhtml絡み。
if(hoge.type == "0"){
if(hoge.isEditable != "1")
html+= '<select class="form-control" id="attribute" disabled="disabled">';
}else{
html+= '<select class="form-control" id="attribute">';
}
html+= '<option value=""></option>';
html+= '<option value="0" selected="selected">文字</option>';
html+= '<option value="1">数値</option>';
html+= '<option value="2">日付</option>';
html+= '<option value="3">時間</option>';
html+= '<option value="4">日時</option>';
html+= '<option value="5">ラジオ</option>';
html+= '<option value="6">プルダウン</option>';
html+= '<option value="7">チェックボックス</option>';
html+= '</select>';
}elseif{hoge.type == "1"){
if(hoge.isEditable != "1")
html+= '<select class="form-control" id="attribute" disabled="disabled">';
}else{
html+= '<select class="form-control" id="attribute">';
}
（一部略）
html+= '<option value="0">文字</option>';
html+= '<option value="1" selected="selected">数値</option>';
（一部略）
}elseif{hoge.type == "2"){
(type7まで略)
}else{
if(hoge.isEditable != "1")
html+= '<select class="form-control" id="attribute" disabled="disabled">';
}else{;
html+= '<select class="form-control" id="attribute">';
}
html+= '<option value=""></option>';
html+= '<option value="0">文字</option>';
html+= '<option value="1">数値</option>';
html+= '<option value="2">日付</option>';
html+= '<option value="3">時間</option>';
html+= '<option value="4">日時</option>';
html+= '<option value="5">ラジオ</option>';
html+= '<option value="6">プルダウン</option>';
html+= '<option value="7">チェックボックス</option>';
html+= '</select>';
}
だーかーらーーーー！
同じことを繰り返し書くのは愚の骨頂だってばー！
(else)if句ごとに毎回おんなじ内容書いてあると気味が悪いと思う感触は僕だけなのだろうか。
ここでは便宜上html+=で書いてるけど、実際は生で吐いてる箇所でした。
（追記）上記の為afterの実体は結構違うんですが、対処的にはこんな感じで行数削れっていう指示を出した内容として書いてます。
var selected_string = ' selected="selected"';
var selected_0 = (hoge.type == "0") ? selected_string : ''; // コメの指摘を見つつ条件演算子に修正 以下同
var selected_1 = (hoge.type == "1") ? selected_string : '';
var selected_2 = (hoge.type == "2") ? selected_string : '';
var selected_3 = (hoge.type == "3") ? selected_string : '';
var selected_4 = (hoge.type == "4") ? selected_string : '';
var selected_5 = (hoge.type == "5") ? selected_string : '';
var selected_6 = (hoge.type == "6") ? selected_string : '';
var selected_7 = (hoge.type == "7") ? selected_string : '';
var disabled = (hoge.isEditable != "1") ? ' disabled="disabled"' : "" ;
html+= '<select class="form-control" id="attribute"' + disabled + '>';
html+= '<option value=""></option>';
html+= '<option value="0"' + selected_0 + '>文字</option>';
html+= '<option value="1"' + selected_1 + '>数値</option>';
html+= '<option value="2"' + selected_2 + '>日付</option>';
html+= '<option value="3"' + selected_3 + '>時間</option>';
html+= '<option value="4"' + selected_4 + '>日時</option>';
html+= '<option value="5"' + selected_5 + '>ラジオ</option>';
html+= '<option value="6"' + selected_6 + '>プルダウン</option>';
html+= '<option value="7"' + selected_7 + '>チェックボックス</option>';
html+= '</select>';
これで９パターン（初期値が０～７で８回＋どれでもない場合）ものif文で書く必要ないよ！やったね！
（追記）コメントにて、項目名も配列にして回せるという指摘をいくつかいただきましたので、そのうちの一つを反映しておきます。ありがとうございます。
var options = ['文字', '数値', '日付', '時間', '日時', 'ラジオ', 'プルダウン', 'チェックボックス'];
var disabled = (hoge.isEditable != '1') ? ' disabled="disabled"' : '';
var html = '<select class="form-control" id="attribute"' + disabled + '>';
html += '<option value=""></option>';
options.forEach(function(e, i) {
var selected = (hoge.type == i) ? ' selected="selected"' : '';
html += '<option value="' + i + '"' + selected + '>' + e + '</option>';
});
html += '</select>';
##我思う
同じことを二度三度書くのはだるいので（コピペならあまり手間はないが）、エレガントに最小回数にまとめてしまいたい、というサボり？意識を常に持ってアンテナを張りたいなぁと。
###単独記事で書こうとした話ちらっと
マネージャーと話したとき言われたことなんですが、
「君は楽するために【既存コードをよく読みこんで流用したり使いまわしたりで】自分で書くところを少なくしてこなすことを考えてるけど、経験の浅い人は楽するために【既存のとか他人のコードを読まずに】自分でゴリゴリ書いてしまう傾向があるような気がするから、その差じゃないかな？」
かなり思うところはある。まぁ、「長く書いてあるコードは読むのだるいし理解に時間かかるから、未来のために短く書いとこうぜ」っていうのが本音ですが。
#番外編
前回から今回までの間に見かけてしまった悲しいタイポ
##英語ミスった？（orタイポ）パータン
###exist→exit
これ、"exitFlg"ってなってたせいで本当に真逆の意味にとりかねない状態にあって、首を傾げた一分後に頭を抱えました…
初見で脱出(=return)判定してると思ったら、（条件適合要素が）存在してた場合前処理するっていう判定だったというアレ。久々にモニタ殴りたくなった。いや、"isExit"って書いてあるならまだ何とかなる気もするけど（？）。
#つづくの？
アレなコード見かけて怒ったら落ち着く（そして記事稼ぐ）ためにネタにする
（いや、直した先の僕のコードもたぶんあまり質が良くないと思うけど）。