Help us understand the problem. What is going on with this article?

かけだしのフロントエンドエンジニアがもらったレビューを振り返る

みなさん、初めまして。mashuです。

playgroundのアドベントカレンダー17日目です。

かけだしの僕が javascript でもらったコードレビューを振り返ってみたいなって思います。


変数名はわかりやすく

// 変更前
colorValue

// 変更後
hogeColor

全人類の悩みの種ですね。
hoge の色を指定したいときに、colorValueという命名ではなんの色か全くわかりません。
hoge の色であることを誰が見てもわかるようにhogeColorにすると直感的にわかるようになります。

javascript は作りたい機能をもとにコードを書くことが多いと思うので、その変数が何に使われるかを意識して命名したいですね。


アロー関数を使おう

// 変更前
function fuga () {
  console.log("piyo");
}

// 変更後
const fuga = () => {
  console.log("piyo");
}

javascript 初心者の僕はアロー関数という超便利なものを使ってませんでした。
アロー関数を使いたいけど書き方が function と全然違うから…って思ってる人は関数宣言から関数リテラル、function からアロー関数が2つ一気に変わっていてごっちゃになってるかもです。(僕も最初はごっちゃになってました。)
なので、まずは関数リテラルから調べてみて、次にアロー関数というふうに段階を踏むとわかりやすいと思います。


ネストを浅く

const list = ["hoge", "fuga", "hoge"];

// 変更前
if ( list.length === 3 ) {
  if ( list[0] === list[2] ) {
    console.log(list);
  }
}

// 変更後
if ( list.length !== 3 ) return;
if ( list[0] === list[2] ) {
  console.log(list);
}

ネストは2段を超えると読みづらくなります。ネストを浅くできるところは他の表現がないか、ちょっとだけ考えてみたらいいと思います。
三項演算子を使うことで、if文が1文で完結することがあるので、三項演算子も使いたいですよね。


共通部分は一回で書く

// 変更前
isHoge ? console.log("hoge") : console.log("fuga")

// 変更後
console.log( isHoge ? "hoge" : "fuga" )

三項演算子は書き方次第ではコードが読みづらくなります。
共通部分を一回で書くようにすると、読みやすく短いコードになると思います。


マジックナンバーを使わない

// 変更前
$("button").on("click", event => ({
  const buttonWidth = $(event.currentTarget).css("width")
  $(event.currentTarget).css("width", `${buttonWidth + 25}`);
});

// 変更後
const extendWidth = 25;

$("button").on("click", event => ({
  const buttonWidth = $(event.currentTarget).css("width")
  $(event.currentTarget).css("width", `${buttonWidth + extendWidth}`);
});

自分ではなんの数値か分かっていても、レビューする人はなんのための数値かわからないので、なんのための数値かわかるような変数名に代入したらいい感じになります。


ifに空文は絶対にダメ

// 変更前
if ( a === 1 && b === 1 ) {
  ;
} else if ( a === 1 ) {
  console.log("hoge");
} else if ( b === 1 ) {
  console.log("fuga");
} else {
  console.log("piyo");
}

// 変更後
if ( a === 1 && b !== 1 ) {
  console.log("hoge");
} else if ( a !== 1 && b === 1 ) {
  console.log("fuga");
} else if ( a !== 1 && b !== 1 ) {
  console.log("piyo");
}

空文は、コードを書いてるときはわかりやすいですが、レビューしている人は ん??? ってなるし、無駄なif分岐が増えるのもよくありません。
上手にif分岐をして、空文の使用を避けるといいと思います。


最後に

案件に参加させてもらって、コードは自分がわかるだけじゃダメなんだなぁと思いました。
読みやすいコード書けるように頑張ろ。

p.s. 次回リーダブルコード編でお会いしましょう

Why not register and get more from Qiita?
  1. We will deliver articles that match you
    By following users and tags, you can catch up information on technical fields that you are interested in as a whole
  2. you can read useful information later efficiently
    By "stocking" the articles you like, you can search right away
Comments
No comments
Sign up for free and join this conversation.
If you already have a Qiita account
Why do not you register as a user and use Qiita more conveniently?
You need to log in to use this function. Qiita can be used more conveniently after logging in.
You seem to be reading articles frequently this month. Qiita can be used more conveniently after logging in.
  1. We will deliver articles that match you
    By following users and tags, you can catch up information on technical fields that you are interested in as a whole
  2. you can read useful information later efficiently
    By "stocking" the articles you like, you can search right away
ユーザーは見つかりませんでした