LoginSignup
29
29

More than 5 years have passed since last update.

3年目の若手がエスアイヤーのコードをリファクタしようとした

Last updated at Posted at 2015-06-08

オフショアからの納品物を仕様変更で改修することになった3年目社員の抵抗

改修ついでにリファクタしたもの

1行ずつコメント

//hogeを2回行う
hoge(); //1回目
hoge(); //2回目

やたらコメントが多い(※あとで気付いたけど、顧客に単体試験のエビデンスでソースコードをスクリーンショットして提出しようとしてたからかも)。
そのくせ変数にはコメントついてなかったりする。

対応

何もしませんでした → 直しました。
見りゃわかるような意味のないコメントをがっつり削除。
逆にコメントが欲しいところは日本語的に適切なコメントを入れた。
(全くプログラムできない人間にもむりやりプログラム書かせるのでコメント無しにするわけにはいかないのです)

クラスの最初に”グローバル変数”

public class PluginHoge extends hugahuga {
    /*-----------------------------------
            グローバル変数
    ------------------------------------*/
    /**
     * フレームワークのメソッドに渡すために使うなんかのフラグ
     */
    private final int NANIKANO_FLG = 1;

つっこみが追いつかないくらい嘘つかれてる気がする。

対応

別のソースに書いてあったMember Variablesに書き換えておいた。
できるだけその変数を使うメソッドの直前とかできればその中に移動した。

消せないコメント

modの履歴とか不要になった処理とか、変更前のコードとか。

対応

変更履歴をコメントで書くのやめてくれって思う。
必ず今後書き直す人が迷うと思うし、バージョン管理が無い時代に編み出された不要なコストなので、古い慣習タヒねって想いでがっつり消してコミットコメントに書いておいた。

ローマ字変数

UKETSUKE_FLGとかHUMEITENCDとか。

対応

変数名に日本語使うのはスネークケースでもキャメルケースでも謎の迷いがおきる。何よりかっこ悪い。
けど自分の担当特有のものだけじゃないので混乱を避けるため放置。

コメントの無い変数

一行一行コメントを書いてた生真面目さは急にどこかに消えた。

対応

名前をわかりやすいもの(booleanだったらisHogeみたいな)に変えたりコメントで説明足しておいた。

if elseでtrue false

boolean flg;
if("何かの判定") {
    flg = true;
} else {
    flg = false;
}

if(flg) {
    hogehoge();
}

(;゚□゚)ってなった。

対応

if("何かの判定") {
    hogehoge();
}

に変えておいた。改修を見越してたのかもしれないけど、きっともっと良い方法がある。

繰り返しコード

パラメータだけ変えて同じロジック繰り返したり。
あと共通処理をいじりたがらない人にコピペを強要されたり。

対応

メソッドにした。
コピペはやり方だけもらって自分の書き方に落とし込んだ。

共通関数に業務ロジック

突然業務ロジックが出てくると思い通りに動かない。配列を渡した場合全部処理するだけみたいな単純な処理にしてほしい。

対応

文句言いまくって直してもらった。案の定仕様変更入ったし、できるだけ単純化した方がいい。

Javadoc

//なにかの処理
/*
@param a
@return
*/

・・・

対応

書き方知らない人が書いたみたいなので修正した(僕も勉強中だけどな!)。
doc生成しておかしなところとかを共通処理含めとりあえず全部書き直した。
あとJavascriptにもJSdoc書いた。Eclipseならきっと拾ってくれる!

インデント

派手にずれていたり、スペースとタブが混在してたり、不要な改行が続いたりなので最初にやっちゃう。

Checkstyle

タブ派とかインデントのチェック外してためしにやってみたけど真っ黄色になった。

まとめ:何を直したかったのか

オフショアしてるのに日本の社員がソースがっつり書き直さなきゃならないとかアホらしいって思ってやりました。
変な習慣を直したかった。
てゆうか誰かコードレビューする習慣入れて。。

追記

やってみてわかったこと

  • わかっててもみんな直す余裕がない
  • 誰もやり方に疑問を持っていない
  • チームメンバが全員プログラムできるわけではない
  • できる人でも変な書き方を踏襲してしまう
  • コピペプログラマにコピペさせないようにすることは徹底できない(納期が厳しいのに仕様変更が頻繁にある。共通関数などの開発が間に合わない。使用してるフレームワークの固有機能に興味がない etc...)
  • 変えたらメールで使い方とかサンプルコードを周知しないとならない
  • 詳細設計書.xls やテスト項目表.xls やエビデンス.xls を作ったりしながらこれらをやる時間がない
  • 原価率が<censored>
  • 仕事辞めよ
29
29
1

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
29
29