１つ目のジョブが成功なら２つ目のジョブを実行するという処理を
何の疑問も持たずにこんなコードで書く人達がいる。

public enum Result {
	SUCCESS,
	FAILED
}

public class TaskExecuter {
  public void execute(Job job) {
     job.firstTask();

     if( job.getResult() != Result.FAILED ) {
        job.secondTask();
     }
  }
}

これはダメなコードです。さて、何がダメでしょうか。

エンハンスによりEnumがこうなったらどうでしょう。

public enum Result {
	SUCCESS,
	FAILED,
	SUSPEND
}

そう、もう期待通りには動きません。
前の処理が終わっていないのに次の処理を実行してしまいます。
なぜこんなコードを書いてしまうのでしょうか。

それは「失敗でなければ成功」という思い込みですね。
失敗でなければ成功などとは考えてはいけません。
「失敗でなければ失敗していない」以上の何物でもないのです。

私の経験上、思い込みというのはバグの発生源としてかなりの勢力を持っています。
また、バグが見つけられない理由としても一大勢力を築いています。

そんなわけでこう書くべきなのです。

if( job.getResult() == Result.SUCCESS ) {
   job.nextTask();
}

成功したとき以外には動いてもらっては困るのですから成功したら実行と素直に書けばよろしいのです。

同じようにこれはダメです。

if( job.getResult != Result.SUCCESS ) {
  throw new SomeException("errorだよ");
}

答えを曖昧にする日本人なのになぜかプログラムでは中間状態を無視してしまうような思考をするのは不思議ですね。

せっかくなので先日書いたゴミサンプルをC++で書き直してみた。

#include <iostream>
#include <string>

class SampleClass {
public:
	int i;
	std::string s;
};

class Test {
	public:
		SampleClass* methodA(SampleClass* a, SampleClass* b);
};

int main(int argc, char* argv[])
{
	SampleClass a = SampleClass();
	a.i = 1;
	a.s = "a";

	SampleClass b = SampleClass();
	b.i = 2;
	b.s = "b";

	Test t = Test();
	
	std::cout << "== before ==" << std::endl;
	std::cout << "a= " << a.i << ":" << a.s << std::endl;
	std::cout << "b= " << b.i << ":" << b.s << std::endl;
	
	SampleClass* c = t.methodA(&a, &b);

	std::cout << "== after ==" << std::endl;
	std::cout << "a= " << a.i << ":" << a.s << std::endl;
	std::cout << "b= " << b.i << ":" << b.s << std::endl;
	std::cout << "c= " << c->i << ":" << c->s << std::endl;
	
	delete(c);
	return 0;
}

SampleClass* Test::methodA(SampleClass *a, SampleClass *b) {
	a->i = 11;
	a->s = "A";
	
	SampleClass b2 = SampleClass();
	b2.i = 12;
	b2.s = "B";
	b = &b2;

	SampleClass* c = new SampleClass();
	*c = b2;
	return c;
};

同じように作ったのだから結果は当然同じ。
ただ、戻り値cはnewしたものなので最後にdeleteしている。
この辺はJavaではそんなに意識しないことだろう。

== before ==
a= 1:a
b= 2:b
== after ==
a= 11:A
b= 2:b
c= 12:B

しかし超久しぶりにC++を書いたのでアクセス修飾子ってどうするんだっけ？と迷ってしまった。。
昔は当然と思っていたのに宣言と定義を分けるというのもかなり煩わしく感じた。
“System.out.println”と”std::cout <<" を比較すると"std::cout <<"などはプログラムを知らない人には謎の暗号にしか思えまい。。 久しぶりにC++のコードを書いて思ったのはポインタを使いたいとは思わないが、やはり概念は知っておくべきということ。 Javaで参照がどうとか勉強するよりもC++やCでポインタを学んだ方が近道な気がしている。 Javaにはガベッジコレクタがあると言ってもメモリの無駄な消費はメモリの断片化が起こってガベッジコレクタの頻発などパフォーマンスに悪影響です。インスタンスの無駄な生成自体もコストがかかりますしね。やはりJavaであってもいざというときはメモリを意識したプログラムを書くべきです。 こんなコード書いてもいいこと何もないですよ。 [java] SomeClass a = new SomeClass(); SomeClass b = new SomeClass(); // このインスタンスが作られてすぐ捨てられる b = a; [/java]

まずは以下のjavaのコードをみてもらおう。

class SampleClass {
  public int i;
  public String s;
}

public class Test {
  static public void main(String[] args) {
    Test test = new Test();


    SampleClass a = new SampleClass();
    a.i = 1;
    a.s = "a";
    
    SampleClass b = new SampleClass();
    b.i = 2;
    b.s = "b";

    System.out.println("== before ===");
    System.out.println("a= " + a.i + ":" + a.s);
    System.out.println("b= " + b.i + ":" + b.s);

    SampleClass c = test.antiPattern(a,b);

    System.out.println("== after ===");
    System.out.println("a= " + a.i + ":" + a.s);
    System.out.println("b= " + b.i + ":" + b.s);
    System.out.println("c= " + c.i + ":" + c.s);
  }

  public SampleClass antiPattern(SampleClass a, SampleClass b) {
    a.i = 11;
    a.s = "A";

    SampleClass b2 = new SampleClass();
    b2.i = 12;
    b2.s = "B";
    b = b2;

    SampleClass c = new SampleClass();
    c = b2;
    return c;
  }
}

mainメソッドでantiPatternメソッドを呼ぶ前後で変数値を出力している訳だが何が出力されるか分かるだろうか。

結果はこうである。

== before ===
a= 1:a
b= 2:b
== after ===
a= 11:A
b= 2:b
c= 12:B

変数cは良いとしても、aやbまでもmainメソッドだけを見ていては決して分かりようがない結果が出力されるのである。さらに言うとantiPatternメソッドを見てみると直感的にはbも更新されてよさそうな気がするのだが実際はそうはならない。この辺はjavaのメソッドはポインタの値渡しであるという言語仕様によるものである。

しかしこれは言語仕様が許しているからと言って書いてはいけないコードの代表格と言えるだろう。mainメソッドを読んでいるのにantiPatternメソッドがどうなっているかまで意識しなくてはいけない。

上の例ではantiPatternメソッドが実行ステップにして10行足らずで完結しているのでまだよいが、antiPatternメソッドも同じように別のメソッドAを呼びだしてAの中でも渡した変数の中身が変わって返ってくるとなったらどうだろうか。さらにAの中でもBを呼び出し、Bの中でもCを呼び出し・・・。
結局全部のコードを理解しないと高々数十行のmainメソッドすら読み解くことができない。
これでは保守性が悪くてどうしようもない。
これをゴミコードと呼ばずにいられるだろうか。いや、いられまい（反語）。

さて、突然こんなことを書いたのは仕事でこんなコードを見たからである。
実際はもっと処理があるが、ポイントだけ抜粋した。

public class Y {
  public void methodA() {
    ・・・
    XListGenerator generator = new XListGenerator(a,b,c);
    List<X> xList = new ArrayList<X>();
    ・・・
    List<Something> somethingList = new ArrayList<Something>();
　　・・・
    generator.set(xList, somethingList);
  }
}

クラス名が悪い、変数名が悪いとか言いたいわけではない。
methodAが何をやっているか分かるだろうか。
「generatorに２つのlistを設定している。」と読みとるのが自然ではないだろうか。

しかしgenerator.set(list,somethingList)の中を見て愕然とした。以下のような内容なのであった。

   public set(List<X> xList, List<Something> somethingList) {
       for(i=0; i<somethingList.size(); i++) {
          String s= somethingList.get(i).getString();
          xList.get(i).setS(s);
       }
   }

なんなんでしょうかこれ。

完全に依存関係があるんだしメソッドを分けたって百害あって一利なしだ。setの主体もgeneratorなのかなんなのかよく分からない。
これでいいじゃない。

public class Y {
  public void methodA() {
    ・・・
    List<X> xList = new ArrayList<X>();
    ・・・
    List<Something> somethingList = new ArrayList<Something>();
　　・・・
    for(i=0; i<somethingList.size(); i++) {
       String s= somethingList.get(i).getString();
       xList.get(i).setS(s);
    }
  }
}

どうしてもgeneratorを使いたいなら、なるべく原型を残したとしてこうだ。

public class Y {
  public void methodA() {
    ・・・
　　XListGenerator generator = new XListGenerator(a,b,c);
    ・・・
    List<Something> somethingList = new ArrayList<Something>();
　　・・・
    List<X> xList = generator.generate(somethingList);
  }
}

プログラムは動けばよい！というのは自分一人で作っている場合に限るのである。
こんなコードを読まなければいけない他人の身にもなってコードは書くべきである。

うーん。たった今テレビで3.11震災後についてのドキュメンタリーをみていた。
その中で家族を失った人に対して「いつまでも悲しんでいてはいけない。」「悲しみは復興の妨げ」なる空気が生まれているようなことが語られていた。

そんなこと言ったって無理でしょう？感情を抑えろなんて釈迦じゃあるまいし。悲しいもんは悲しいんだもん。

せめて「悲しみを忘れずに生きていこう」「悲しみを忘れる必要はない。それでも生きていかなきゃ。」くらいにしときましょうよ。

という超ひさしぶりのカキコ。