かとじゅんの技術日誌

技術の話をするところ

前向きな人にコードレビュー

Java

IT業界3年目の人で非常に前向きな方とお友達になりましたw
ブログのタイトルからしてぶっ飛んでるんですがwww

勤務時間をテキスト形式で保存

コードを晒すことはすばらしいねー。適当にコードレビューしてみよう。

  • コマンドライン引数は1とか0ではなくてstartとかfinishでわかりやすい方がいいのではないかなーw
  • mainメソッドは基本的には戻り値はintにしたほうがいいよ。System.exitで、正常終了するときに0、それ以外は非0にする。バッチ処理などでJavaプログラムが成功したか失敗したか知ることができるため。
  • mainメソッド内での実行時例外のスローはせずに、エラー情報を標準エラーか、ログファイルに出力して、非0をreturnするとよいよ。
  • 以下の部分はディレクトリでない場合にディレクトリを作っているわけなので、同名のファイルが存在していた場合は例外が発生してしまう。!timeCardDirPath.exists()でないかな。
if (!timeCardDirPath.isDirectory()) {
timeCardDirPath.mkdirs();
}
  • これはいらないような気がする。fos = new FileOutputStream(timeCardLogFile, true);で作成されるんじゃないかと思うけど。
if (!timeCardLogFile.exists()) {
timeCardLogFile.createNewFile();
}
  • これは読みにくい＞＜。if文は1行でも必ず{}をつける。
if (fos != null)
fos.close();
if (osw != null)
osw.close();
if (bos != null)
bos.close();
  • mainメソッドにダラダラ書くのは見通しが悪いのでインスタンスメソッドに機能を実装する。

今のmainの例では手続きが中心になっているので、オブジェクト中心に考える癖を付けたほうがいいね。で、TimeCardクラスが行う責務ってなんだろうと考えよう。どういう機能がありうるんだろうか？TimeCardクラスにappendDateTimeメソッドを作って、現在の出退勤時間を書き込むとか。

public static void main(String[] args){
    try{
        if ( args.length != 3 ){
            System.out.println("引数が不正です");
　　　　　　System.exit(-1);
        }
        File dir = new File(args[0]);
        File file = new File(dir, args[1]);
        TimeCardMode mode = "start".equals(args[2].toLowerCase()) ? TimeCardMode.START : TimeCardMode.FINISH;
        new TimeCard(file).appendDateTime(mode);
    }catch(Exception ex){
        ex.printStacktrace();
        System.exit(-1);
    }
    System.exit(0);
}

public TimeCard(File outputFile){
    this.outputFile = outputFile;
}

public void appendDateTime(TimeCardMode timeCardMode){
    // ディレクトリを作るならこんな感じかな
    if ( outputFile.getParentFile() != null && !outputFile.getParentFile().exists() ){
        outputFile.getParentFile().mkdirs();
    }
    // 以下はmainの処理と同様
}

そもそも、今回の例ではTimeCardクラスとしているわけですが、本当にタイムカードを表しきれているかのかどうかというもの疑問。WorkingTimeWriterクラスにしたほうがいいのかもしれません。要するにこれは命名の問題だね。命名は超重要、命名は。ということで以下をご参照ください。
きれいなソースコードを書くために必要な、たったひとつの単純な事 - よくわかりません

ぜひ、がんばってください。

追記:
命名では、キチンとした英単語を使おう。ということでALC(英辞郎でもよい)で 日本語を入れると英単語や英文の文例が出てくるので、一番自然な英単語を命名に使う。
http://www.alc.co.jp/