チームリーダー的な立場でのコードレビューが難しいなという話

お久しぶりです。日記です。

今日から喫茶店に入ってパソコン開いたら記事を書くみたいな決まりでやっていこうと思います。

今日は秋葉原のJR電気街口にあるBECK'S COFFEEで書いています。以下、本文。

半年前は業務委託、今は6月ぐらいから正社員としてまた違う会社で働いている。
で、今まで会社を割と転々としてきたせいで後輩というか「自分より経験の少ない人と一緒に働く」という経験をまともにしてこられなかったという経緯があるんだけど、今の会社だと自分がチームの中で一番コードも書けて知識があるので（これは驕りでもなんでもなく自分以外はほぼアルバイトの学生だったりするため）、技術選定とかコードレビューの柱の役をやるという役目を任されることになった。

そうなると、必然的にコードだけガリガリ書いていればよいというわけには行かなくなってくるわけだけど、とはいえそれでもコードは書かないといけないので、このバランス感覚が結構難しい。
単に仕事量が増えてつらいという話ではなく、相手にどの程度のクオリティを求めるべきなのか、という話である。

先に注釈した通り、僕が見るのは主にアルバイトの学生さんの書いたコードで、ストレートにいえば僕のコードに比べてあまり質の良くないコードが上がってくる。まあ、これ自体は仕方がない。

僕が悩んでいるのは、それについてどこまで「もうちょっと頑張って」っていうべきなのか、ということである。

例えば、僕の基準でスーパーキラキラスマートなコードが書けた時を100点ぐらい（そういうことは自分でも滅多にないけど）だとして、他人にその基準を押し付けるのはさすがにやべーやつなのでまあ70点ぐらいのコードならLGTMを出すのが妥当なんじゃないかと思う。

で、そういう基準のところに60点のコードのプルリクエストを出された時（いや、自分の中でそのコードが60点だと思えるかどうかはまた別の話なんだけど）、果たしてどの程度直させるべきなのか・・・。

単純に「何が減点ポイントなのか」を明確に示してそれで合格点になればそれでよいんだけど、例えば「そもそもここ全部書き換えて欲しいなぁ」みたいなときがあり、それでもせっかく動いてるコードを直させるのは心情的にも工数的にも申し訳なさが勝ってしまうので、そこから「ここを直せたらプラス5点」みたいな指摘しか結局出来なかったりする。

例えば以下のような、DBからデータを集めて配列としてクライアントに返すAPIのコードがあったとする。

const articles = articleRepository.getItemsByPeriod(userId, fromDate, toDate);
const articleIds = [];

for (const article of articles) {
  articleIds.push(article.id);
}

const comments = commentRepository.getItemsByArticleIds(articleIds);
const result = [];

for (let i = 0; i < articles.length; i++ ) {
  result.push({
    title: articles[i].title,
    content: articles[i].content,
    // ここで、comments は対応する記事のデータが過不足なく取れるものとする
    commentList: comments[i].commentList
  });
}

return result;

要するにユーザーIDから記事データを引っ張ってきて、そこにコメントをくっ付けてJSONとして返すやつだと思ってもらえればいい。
(JOINしたらよくない？みたいな話はリレーショナルなDBじゃないとかで出来ないってことにしておいて)

正直、こういうコードを出されると僕の中でちょっと減点してしまう。

これは僕ならこう書く。

const articles = articleRepository.getItemsByPeriod(userId, fromDate, toDate);
const comments = commentRepository.getItemsByArticleIds(articles.map(article => article.id));
const resutls = articles.map((article, index) => ({ 
  title: article.title,
  content: article.content,
  commentList: comments[index].commentList
}));

return results;

正直、前者のコードのほうが分かりやすいし読みやすいという人もいるだろう。でも、僕は 後者のほうが読みやすい と感じる。

さて、こういう場合に「こう書いたほうが好きなのでこう書いて下さい」と言ってPRコメントに後者のコードを書くべきかどうかにいまいち自信がない。

前者のコードでも間違いなく動くし、アルゴリズムも 計算量 も何ら変わりがない。単に書き方の問題である。

工数的に考えればこのPRはさっさと Approve してマージしろという話なのだが、いずれこのコードを自分でメンテナンスしなければならなくなったときに、前者のコードを触ろうとするときに、その、なんというか、 ものすごいイライラしてしまう

書いた人に対するヘイトを向けているわけではなく、単に「うげええ～～～～～～！！！これ嫌い～～～～～～～～！！！」となってしまう。
でも、ここでもし「こう書いたほうが好きなのでこう書いて下さい」と言ってPRコメントに後者のコードを書いたらそれこそヘイトを買うだろう。とても悪くいうとコードの私物化に近いのではないか。

とはいえ、プロジェクトの最初の頃はそう思って↑みたいなコードをマージし続けていたツケが（メンテナンス性な意味で）今回ってきていて、やはりここは「うげええ～～～～～～！！！これ嫌い～～～～～～～～！！！」という感情に社会性フィルターを通して「こう書いたほうが好きなのでこう書いて下さい」というべきだったのかもしれない。。

世間のリーダーはどこで折り合いをつけているのだろうか、気になるので書いてみたという日記でした。

お気持ち表明でまとめもクソもないので尻切れトンボ感はありますが、以上です。