プログラミングのバグや無駄を減らすために初心者や新人に意識してほしいこと4つ

このページに書いてあること

仕事でも自習でもいいのですが、プログラミングをするときに「これを気をつけておけば無駄なミスを減らせる」というものを書きます。
(よく別のサイトである様な勉強方とかそういうのではありません。コードの書き方の話です。)

前提

例示はJava
Java以外のプログラムでも考え方は全く一緒

気をつけて欲しいポイントは、以下の通りです。

文字列比較では定数を前に持ってくる
処理の結果を判定する為に数字や文字列を使用しない
文字列をなんでもかんでも定数化しない
意味のないコメントを書かない

・文字列比較では定数を前に持ってくる

文字列比較と題しましたが、要は

String.equals()メソッドによる比較はNullpointerExceptionを発生させる可能性あるから気をつけてね。

という事です。例えば以下の様なコード

public void sample(){ 
 String text1 = "AAA";
 String text2 = "BBB";
 
 System.out.println(text1.equals(text2));
}

実行した場合は標準出力にfalseと表示されます。当たり前ですね。
これは良く教本とかで見るコードだと思います。

このコードの内、text2の方をフィールド定数に変更します。

private String TEISU = "BBB";

public void sample(){ 
 String text1 = "AAA";
 System.out.println(text1.equals(TEISU));
}

これも問題なくfalseです。必ずfalseになります。

ではここで、text1をメソッド引数から取得する様にしたらどうなるでしょうか。

private String TEISU = "BBB";

public void sample(String text1){ 
 System.out.println(text1.equals(TEISU));
}

上記のコードでは、text1にnullが渡されてしまえばヌルポが発生します。そして、text1にnullが来ないことは保証されていません。

だからこのコードはヌルポを潜在させてしまうのです。で、たまにこうする人がいます。

private String TEISU = "BBB";

public void sample(String text1){
 if(text1 == null){
  text1 = "null";
 }
 System.out.println(text1.equals(TEISU));
}

要は、nullだった場合に何か別の処理を入れて、ヌルポを防ごうという話です。いや、そうするんだったら、

private String TEISU = "BBB";

public void sample(String text1){
 System.out.println(TEISU.equals(text1));
}

の方がよくないでしょうか？TEISUは必ずBBBが入っているのでnullでない事が100%保証されています。この状態でtext1にnullが来ても、BBBとnullを比較するのでfalseが帰ってくるだけです。
これは文字列の比較の例で書きましたが、DTOなどのオブジェクト同士の比較においても、nullでない事が保証されている側を前に持ってきてください。

・処理の結果を判定する為に数字や文字列を使用しない

問題点の理解は比較的簡単だと思います。DB接続処理を例とした以下のコードをみてください。

//接続失敗
int CONNECT_ERROR = 90;
//接続タイムアウト
int CONNECT_TIMEOUT = 91;
//受信タイムアウト
int RECIVE_TIMEOUT = 92;
//登録失敗
int INSERT_FAILD = 93;
//更新失敗
int UPDATE_FAILD = 94;
//処理成功
int SUCCESS = 0;

public int insertUser(String userId){
 DB接続処理いろいろ
　　　・
　　　・
　　　・
 if(DB接続に失敗していたら){
  return CONNECT_ERROR;

 }else if(接続タイムアウトしたら){
  return CONNECT_TIMEOUT;

 }else if(受信タイムアウトしたら){
  return RECIVE_TIMEOUT;
 }

 登録の成否を確認
 if(登録に失敗していたら){
  return INSERT_FAILD;
 }

return 0;
}

こんなコードがあったとします。別に問題はない様に思えます。

はい。この「処理の結果を判定する為に数字や文字列を使用した場合」の弊害はこのコード自体にはありません。弊害を受けるのは、このメソッドを呼ぶ側です。

先ほどの「insertUser」メソッドをユーザー情報を登録したいクラスが実行したとき非常に困ります。

このメソッドの処理結果に何が返ってくるかわからないですよね。
だってメソッドのIFで見ると、返ってくるのはただの数字ですから。

intの表現範囲分の戻り値の可能性があります。

ではどうするか。

各種処理の失敗はそれを示す例外をスローするようにするか、Enumを返すようにしましょう。
これにより、プログラミング時の制約としてメソッドの戻り値を縛ることができるようになります。

返す値が無いメソッドはvoidで結構です。無意味な値を返す必要はありません。

・文字列をなんでもかんでも定数化しない

マジックナンバーは良くないけど、文字列は別に定数化しなくていいよという話。

単純な話です。以下のコードを見てください。

public void execute(){

 String str = new String[32];

 for(int = 0; i < 100; i++){
  str[i] == "header-name:"+i;
 }
}

この中で、マジックナンバーだから定数化しろ言われがちなのは２つです。
数字の32と文字列のheader-nameです。

で、32の方は何を意味する32なのかがわからないので完全なマジックナンバーとなり定数化すべきです。
例えば、KEY_SIZE=32という定数なら鍵のサイズが32なので、それを意味する32であることがわかります。

では一方、header-nameという文字列はどうでしょうか。
これを定数化しようとしたら、HEADER_NAME="header-name"とかになりませんか？それにこの文字列、読めばheaderの名前を示しているんだとわかります。
意味無くない？？

今まで見た中で一番感心したのは、

public static String COMMA = ",";

ですかね。

というわけで意味のない定数は用意する必要はないということになります。

ここで反論として以下のことを言ってくる人がいます。

「修正するときに修正範囲がわからなくなるじゃん」

ある観点では一理ありますが、ここで重視したいのは「【意味のない】定数化はやめよう」です。
言い換えれば「文字列を全て定数化すべき」という考えからの脱却です。

定数化してたとしても、header-nameという文字列が一般的だった場合、定数定義を買えたところで意図したところに修正が効くかは不透明です。その定数を誰が使っているかを調査しないといけないためですね。

・意味のないコメントを書かない

これも何かの宗派かと思うほど一定数いるんですけど、コード１行１行に処理を説明するコメントは絶対に書かないでください。

それ、平仮名の上にカタカナで読み仮名降ってるのと同じですよ。という話です。

以下のコードを見てください。

//StringBuilderを用意
StringBuilder sb = new StringBuilder();
//arraysの数だけ繰り返す
for(String str : arrays){
//切り出した文字列を表示
 System.out.println(str);
//StringBuilderに追加
 sb.append(str);

//もしStringBuilderの文字数が100文字を超えたら
 if(sb.toString().length > 100){
//ループ中断
  continue;
 }
}
//生成した文字列を返却
return sb.toString();

このコメントにどんな価値があるのでしょうか？

私には、コードとコメントで同じことを言っているのでただ２重管理になってしまっているだけにしか見えません。

コメントなくてもコード見りゃわかるよ・・・

という内容はわざわざ書かなくていいよねということです。

さて、ではコメントは不要でしょうか？NOです。必要なコメントがあります。

プログラムの意味を教えてあげるコメントは絶対に必要です。

先の例では以下のよう１行のコメントがあれば十分でしょう。

StringBuilder sb = new StringBuilder();
//100文字以上になるまで繰り返し連結
for(String str : arrays){
 System.out.println(str);
 sb.append(str);

 if(sb.toString().length > 100){
  continue;
 }
}
return sb.toString();

この程度のコメントであれば、そもそも不要ですね。

コメントは、処理のまとまりに対して「やりたいこと」や「なぜその処理が必要なのか」というコメントを書きましょう。

例えば、一生懸命ビットを操作しているようなソースコードがあった時に、

ビット配列分繰り返す
1ビット右にずらす
取得したビットを1000で割る

なんてコメントが書いてあっても、それが正しいのか自体が判断できません。

それらの処理のまとまりに対して、

すべてのビット配列の各桁を1000で割る

という感じのコメントであれば、何がしたいのかは分かるようになるので、コードが正しいかどうかの判断も付きやすくなりますね！