4
1

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

後輩にレビューで突っ込みを入れていたらスパゲティができたでござるの巻

Last updated at Posted at 2017-09-05

#はじめに
まだ事態が落ち着いてないんですが、半自戒、半怒りの整理メモ。
反面教師事案としてお使いください。できるだけ愚痴にはならないように気を付けてはいますが、一部見苦しかったらすいませぬ。

#何が起こった
以前までに書いたような感じで細々とレビューを入れてパスタ化を回避し、
できるだけ可読性の高い(=メンテナンスが楽な)コードができるように務めてたんですが。

その後輩が一ヶ月分まるっとまとめてコミットしやがりまして、
しかもそのコードがすさまじいまでのスパゲティになっていて久々に堪忍袋の緒が切れました。

##原因
いくつか考えられますが、できるだけ後輩を責めないように分析。

###素養
そもそもPGとして確かにそこまで高くなかったのですが、
そこはチームなんで適宜フォローしてコードレビューとかを通して品質を上げてきていたわけです。
ただ、どうやら本人は品質を上げるという発想そのものが少なそうです…
(毎回同じようなことを指摘してるのに、延々同レベルで出してくる)

###フォローに入った人との相性
ベテラン(とされていた)人がフォローに入っていたのですが、実はいままでばれてなかっただけの能力不足な上、とにかく自分の責任でないことを確認してから入る「典型的な(僕主観で)昔のクソSE(※)」だと発覚。
※「自分の責任になることを回避するための足場確保」を最優先で確保するタイプ、って言いたいのです
PMさんに対しての弁解が「それは後輩君の担当だから」的な責任逃れに終始したせいで、PMさんが僕以上に激おこスティック略だったのがとても印象的。なお僕よりはるかに年上で経歴長い。

###隠ぺい体質
いわゆる進捗確認に対して「順調です」「問題ありません」と言いつつ、足の進みが遅いタイプ。「実際はそれ問題だよね」というPMの追及が入るまでその意識がなかったのが。
素養に近い部分もあるのですが、これはまぁ場数が少ない若い子なら単独では仕方ないと思う部分で。

むしろ問題はフォローに入った人が(フォロー入り後の進捗遅れが)自分の責任になるのを恐れて
更に進捗を隠していたというのが決定打。何のためのベテランフォローだとおもってんだ、ひどいシナジーだ。

###夏休み
うちの夏休みはずれて取っているという不運もあって、その期間その気になれば怠けたい放題であり。

不運にもリードプログラマ(僕です)とマネージャーが交互に一週間ずつ居なかったのですが、その期間に進捗がずれたので、計2週間*2人=事実上の1人月がほぼ無駄になっているという…

###レビューうぜえ
今回決定的に頭を抱えた要素。
「リーダーのレビューがちょいちょい入りすぎて集中できないからまとめて出しました」
どーん。何のためのチームだ。補足しておくと、最初の頃は「後輩君とベテラン(自称)と並行してコードを組んだため、2人のローカル統合に時間がかかっている」っていう説明をもらってたわけで。そもそも何のためのsvnだと思ってんだちまちま上げて駄目なら戻せ。

##私的な反省
別の後輩(こっちは最近無事に卒業してステップアップしました。よきかな。)の面倒を見るのが中心だったのだけど、その子を含めた自分から聞いてくる子のフォロー機会のほうがが圧倒的に多くなったのはやや反省。聞いてこない子ほど不安除去も含めてこっちからちまちま聞くべきで、その際に表面的な「大丈夫?行き詰ってない?」ではなくちゃんと具体的に踏み込んで闇を払いに行くべきだったのかな、と。
※一応マネージャーの仕事と言えばそうだし、僕自身当然自分の開発業務は当然あるのでどこまでやっていいかはアレですが。

#混沌の片鱗
(おまけ。あと微妙に実情とは違います。反面教師としてお使いください)
一例として、updateUserInfo(bool isModeA)というメソッドが呼ばれている箇所。2カ所あり、既存において微妙な動作の違いはメソッド内でbool値を見て吸収していたのですが、今回の更新で処理を切り分ける必要があったために分離したのですが…

##分離後のメソッド名が酷い
"updUserInfo"と"newUpdateUserInfo"。そもそも命名法則規約違反である。

##変数名とか設計もひどい
既存のように(bool)isDeletedとかで処理分ければいいのに、オンオフしかない2モードに対してわざわざ(string)deleteFlgとか書いてたり("1"がオン)。

##使われてない"updateUserInfo"が生き残っている
しかも中途半端に改造された状態で放置されてた。上記のクソなメソッド名もあり、参照元が全部消えていることを確認するのが大変でした…

#なお現状
リリース最優先のせいでスパゲティと化したコードと格闘するために労働時間マシマシ。
なんとかリリース状態にこぎつけてもスパゲティをほぐす作業と次期開発の並走が確定しており。
なお件の自称ベテランはすでに月末での撤退準備を始めている(これも一因なのか)。やりにげだいなみっく。

俺、直していく中で面白そうなネタがあったら、また投稿するんだ…

4
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
4
1

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?