Javaでプログラムを書く際に意識しておきたいこと

以下、個人的にJavaでプログラムを書く際に意識しておきたいことです。

ただし、学術的な裏付けなどがある内容でありません。あくまで私の経験に由来する内容となっています。

そもそもコンテキストによってはそぐわない内容もあると思いますので、その辺はうまいことスルーしてもらえたらと思います。

Collection

空のList

メソッドの戻り値として空（size==0）のListを返却する場面がありますが、その場合はCollections.emptyListを使うのが良いです。
new ArrayList()でListを生成してreturnするよりも、処理も早くコードの意味も分かりやすくなります。
ただし、このメソッドで返されるListはImmutable（不変）であることを理解しておく必要があります。
Collectionsクラスには、空Setや空Mapを返すメソッドも用意されています。

大量データをListに格納する場合はサイズ指定

ArrayListのデフォルトコンストラクタで生成されるインスタンスのデータ格納容量は10です。
この値よりもはるかに大量のデータを格納（add）することが確定している場合は、最初からサイズ指定のコンストラクタを使うべきです。
基本サイズ(10)を起点にしてデータを格納を繰り返すと容量拡張が何度も実行されパフォーマンスに悪影響を及ぼす場合があります。
（C言語でいうところの、mallocをしたあとにreallocを繰り返しているような感じになってしまいます）

EnumMap

Enum(列挙型)をキーにしたMapを作成したい場面がありますが、そのような場合はHashMapではなくEnumMapを使用した方がよいです。
HashMapを使用するよりも実行効率が良くなります。

Vector、Hashtableを使用しない

java.util.Vectorやjava.util.Hashtableは同期化されたCollectionですが、性能などの問題もあり使用しないようにするべきと一般的に言われています。
同期化が必要ない場面ではArrayListやHashMapへの置き換えを行うべきです。同期化が必要な場面でも別の代替手段を検討すべきです。
よほど理由がない限りVector、Hashtableを使用しない方がよいです。

サードパーティのCollectionライブラリ

Java標準のコレクションフレームワークの実装は古いと言われ、問題も抱えています。
（プリミティブ型をそのまま格納できない、メモリ効率が良くない、コレクションの種類が貧弱、、、etc）
標準のCollectionの問題等を解決するために以下のようなライブラリが存在しています（標準のCollectionでパフォーマンスで問題となった場合や使い勝手が悪い場合などに利用を検討するのがよいです）

• Eclipse Collections
• Trove
• fastutil
• Koloboke
• Guava

メソッド

戻り値にnullを使用しない

return nullとなっているメソッドは、メソッド利用側でnullチェックが必要となり、チェックを忘れるとNullPointerExceptionを引き起こす場合があります。
メソッドを作成する際に出来る限りnullをreturnしないように心掛けた方が良いです。
戻り値の型がListなら空Listをreturnする。戻り値の型が配列なら要素数0の配列を返却する。それ以外の場合はNullObject（等のデフォルト動作をするオブジェクト）、Optionalの導入を検討する。
どうしてもnullをreturnする必要がある場合は、javadocコメントにどういったケースでnullがreturnされるかを記述するべきです。

引数でnullをとったら即時にエラーを通知する

一般的なほとんどのAPIにおいて引数にnullを渡されるというのは異常ケースです。（API利用側のバグと言えます）
業務の（publicな）メソッドで引数にnullをとった場合もnull用の特別の業務処理を行うのではなく、即時にNullPointerExceptionやIllegalArgumentExceptionで呼び出し側にエラーを通知するようにした方が良いです。
null値をAPI間で引き回すと処理が分かりにくくなったり、バグが混入しやすくなります。
（もちろん、nullを特別に扱わなければならないケースもあるとは思いますが、そのようなケースは稀です）

マルチスレッド

スレッドセーフではない危険な標準クラス

スレッドセーフだと思って使用するとマルチスレッド環境下で思いもよらない例外が発生するクラスがjava標準で存在します。

• java.text.SimpleDateFormat
  • 有名ですが、SimpleDateFormatはスレッドセーフではありません。staticなメンバとして保持したりすると予想外の動きをします
    対処法としては以下
    • commons-langのFastDateFormatを使用する
    • ThreadLocalにして使用する
    • 処理のたびにnewする
      • ただしSimpleDateFormatのインスタンスを生成（new）するコストはかなり高いです。
• java.text.NumberFormat
  • NumberFormatのjavadocを読むとわかるのですがマルチスレッドでアクセスする場合は外部的に同期化する必要がある旨が記載してあります。
• java.text.DecimalFormat
  • NumberFormatと同様です
• java.text.MessageFormat
  • NumberFormatと同様です

スレッドセーフの表明

JSR305のアノテーションを使用することで対象クラスがスレッドセーフであるか、そうでないかを表明することができます

• @ThreadSafe ・・・ スレッドセーフであることを表明するアノテーションです
• @NotThreadSafe ・・・ スレッドセーフではないことを表明するアノテーションです

（上記アノテーションはこの本に由来する）

例外

例外の翻訳（exception translation）

例外をスローする側とキャッチする側のレイヤーが異なる場合などは特に、抽象概念に適した例外を投げる必要があります。
たとえば、何らかのデータにアクセスするオブジェクトが、SQLException、FileNotFoundException、IOExceptionの３つをスローするよりも
それらをラップしたDataAccessExceptionをスローする方が上位側も扱いがやりやすくなります。
（本件は「Effective Java」の項目61にも同様の内容が書かれています）

複数の例外をスローしない

「例外の翻訳」と同様ですが、１つのメソッドから複数の例外をスローするのは得策ではありません。
そのメソッドを利用する側からすると迷惑でしかありません。極力１つ(or ２つ)の意味のある例外をスローするようにすべきです。
複数の例外をスローするメソッドを利用する側は、java.lang.Exceptionでまとめて例外をcatchをしたくなってしまいます。

• よろしくない例

// 本メソッドの呼び出し側はそれぞれの例外を個別にcatchする必要がある
public ServiceResult doService(ServiceRequest request, User user) throws NotCertifiedException, CertificateException, SQLException, SocketException {
    
    // do something
}

• 改善例

// 本メソッドの呼び出し側は複数の例外を個別にcatchする必要がない
public ServiceResult doService(ServiceRequest request, User user) throws ServiceException {
    
    // do something
}

標準例外を使用する

外部向けのAPI等では特に以下のようなJava標準例外を積極的に使用することでバグを生みにくくなります

クラス名	利用シーン
IllegalArgumentException	引数が想定しない値である場合などにスロー
NullPointerException	（引数などがnullで）null操作が発生してしまう場合などにスロー
IllegalStateException	実行すべき状態が不整合である場合などにスロー
UnsupportedOperationExcption	対象の操作を提供しない場合などにスロー
IndexOutOfBoundsException	範囲外の要素にアクセスした場合などにスロー

チェック例外 vs 実行時例外

Javaで定義する例外は、Exceptionクラスを継承したチェック例外(検査例外)とRuntimeExceptionクラスを継承した実行時例外の２つに分かれます。
チェック例外を定義した場合、例外のハンドリングをしているかどうかをコンパイラでチェックすることができます。
これは一見、例外のハンドリング漏れがなくなるので良い事のようにも思えるのですが、
メソッドで上位に例外をスローしていくような場合は、例外がスローされる通り道となってしまう中間のメソッドにおいてもthrows節に対象例外を記述する必要があり、
これは対象メソッドが関心のない例外によって汚されているとも考えられます。
また、チェック例外はJava以外の言語ではほぼ見られない機構であり、本当に必要なものかは若干不明です。
例外クラスを定義する場合に、何も考えずExceptionクラスを継承するのではなく一旦上記のような問題点等があることを考慮した方が良いです。
Javaのチェック例外という概念がJava誕生からこれまでの歴史をもってしても、今なお他言語や他プラットフォームに普及していません。
チェック例外が評価されていない（扱いにくい等の負の側面が多い）という事実だと思われます。

CloneNotSupportedException

Object#cloneメソッドがスローするCloneNotSupportedExceptionはチェック例外ですが、
スローされる原因はCloneableインタフェースの実装し忘れによるものです。そのような状況はコードとしてのバグと言ってもいいと思います。
また、この例外をスローされた側はcatchしてもほとんどの状況で有用な処理はできないはずです。
cloneメソッドをオーバーライドする場合はCloneNotSupportedExceptionを盲目的にスローするのではなく、
IllegalStateExceptionやRuntimeException等の実行時例外でラップ（またはInternalErrorで例外メッセージをラップ）してスローし、 cloneメソッドの利用側の負担を減らすのが良いでしょう。
（正直、CloneNotSupportedExceptionがチェック例外である理由は基本的にないと考えられます。cloneメソッドはチェック例外をスローしているために扱いにくいAPIとなっている一例だと思います）

UnsupportedEncodingException

String#getBytes等のメソッドで本例外がスローされる場合があります。
CloneNotSupportedExceptionと同様にチェック例外となっていますが、この例外をcatchしても業務アプリケーションとして有用な処理はほぼできないと考えられます。
（そもそも、キャラセットを"UTF-8"等と指定すれば、UnsupportedEncodingExceptionはスローされません。（Javaのプラットフォーム実装は標準の文字セットをサポートするからです））
本例外をスローするメソッドはRuntimeException等にラップしてスローする事を検討すべきです。

java.lang.Exceptionをスローしない

通常の業務処理において、java.lang.Exceptionをスローすべきではありません。
java.lang.Exceptionを投げてしまうと、エラー（例外）の意味がかなりぼやけてしまいます。対象の処理やレイヤーに適応した名前の付いたExceptionをスローすべきです。
自分の作成したメソッドがjava.lang.Exceptionをスローしている場合、いったいどんな場合にその例外がスローされ、どういう意味を上位に通知しようとしているのか考え直してみるべきです。
（フレームワークやプラットフォーム的な特殊な処理においてはjava.lang.Exceptionを敢てスローするケースがあります）

java.lang.Exceptionをキャッチしない

通常の業務処理において、java.lang.Exceptionをキャッチするのは最小限にすべきです。
汎用的に例外を捕捉できるため、java.lang.Exceptionをキャッチしたくなりますが、java.lang.Exceptionをキャッチするということは、RuntimeExceptionのサブクラスもキャッチするということです。
通常の業務処理でそのようなことを意図する場面はかなり少ないはずです。
RuntimeExceptionのJavadocを見ると分かりますがJava標準でRuntimeExceptionの既知のサブクラスはかなりの種類があります。これらを敢て捕捉したくて、"catch (Exception e)"と書いていますか？
（フレームワークやプラットフォーム、呼び出し階層の上位レイヤー、外部システムとの連携を担うレイヤー等においては、java.lang.Exceptionを敢てキャッチするする役割のクラスが必要な場合もあります）

例外を握りつぶさない

呼び出したメソッドから通知された例外を握りつぶす以下のようなコード書くのは避けた方が良いです。

try{
   someMethod(); // IOExceptionをスローする可能性がある
} catch(Exception e) {
    // 何も書かないで無視する（やってはいけない！！）
}

実際に例外が発生した場合にそのエラー内容や原因が全く分からなくなってしまいエラーの解析などができなくなってしまいます。
基本的には例外に対応した何らかの処理、そのまま（またはラップして）上位に例外をスロー、最悪でも例外内容をログ出力して例外を記録した方が良いです。
以下のコードの場合はjava.lang.Exceptionをcatchして無視しているので、バグなどで発生したRuntimeException（のサブクラス）等も握りつぶしてしまい、
アプリケーションとして挙動がおかしくなってしまう可能性すらあります。
（ただし、close処理などで意図的に例外を無視する場合などの理由があるケースは除きます）

バグを意味するRuntimeExceptionをcatchしない

java標準でRuntimExceptionのサブクラスにはコードのバグによって発生するような例外（IllegalArgumentException、IndexOutOfBoundsException・・・etc）が存在します。
そのような例外を通常の業務処理においてキャッチして処理を続行すべきではありません。
例えば、IndexOutOfBoundsExceptionが発生するケースは、C言語ではセグメンテーション違反が発生してまう状況に近いとも言えます。
セグメンテーション違反が発生したら対象アプリケーションは続行できません。
このように考えると、バグを意味する例外を無理やり捕捉して業務処理を続行するのはかなり不自然と言えます。

例外をスローする際はエラーの原因情報を含める

稼働してるアプリケーションで例外が発生してエラーとなった際に、エラー解析を行うためにはログ（に出力されたスタックトレース）が頼りとなります。
エラーを解析する際に頼りとする情報には以下のようなものがあります。

• 例外（例外クラス）の名前そのもの
  • 例外の名前が具体的であればエラーの原因が推測しやすくなります
• 例外に含まれている元の原因となった例外のスタックトレース
  • 元となる例外がわかると本当のエラー発生個所が推測しやすくなります
• エラー内容を意味するメッセージ
  • 例外の内容を補足してくれます。詳細なメッセージが含まれているとエラーの原因が分かりやすくなります。
    エラーの原因情報をできるだけ上位に通知するために、以下の方針で例外をスローした方が良いです
    • 呼び出し元のメソッドで例外が発生し、それをキャッチして新しい例外をスローする場合
      ⇒ 元の例外を新しい例外に含める（元の例外も含めて上位に通知する）
    • 業務的なチェック処理結果からエラー通知する場合
      ⇒ チェック対象の値と何のチェックでNGだったか（何がまずかったのか）をメッセージとして例外に含める
      （※ただしチェック対象の値が巨大なオブジェト（巨大な文字列etc）の場合は含め方に何らかの工夫は必要です）

実行時例外もjavadocに記述する

RuntimeExceptionを継承した例外クラスをスローするメソッドでは、 javadoc の @throwsにその例外を記載しておく必要があります。
ただし、throws節には記述しません。

誰にもcatchされなかった例外のハンドリング

UncaughtExceptionHandlerをThreadに設定することでハンドリングできます

以下はメインスレッドに例外内容をロギングするUncaughtExceptionHandlerを設定するサンプルです。

public class UncaughtExceptionHandlerSample {

    private static final Logger logger = LoggerFactory.getLogger(UncaughtExceptionHandlerSample.class);
 
    private static final UncaughtExceptionHandler UNCAUGHT_EXCEPTION_HANDLER = new UncaughtExceptionHandler() {
 
        /** 
         * キャッチできなかった場合は、とりあえずログ出力する
         */
        @Override public void uncaughtException(Thread thread, Throwable throwable) {
            logger.error("キャッチできない例外発生", throwable);
        }
 
    };
 
    public static void main(String[] args) {
        // メインスレッドにUncaughtExceptionHandlerを設定
        Thread t = Thread.currentThread();
        t.setUncaughtExceptionHandler(UNCAUGHT_EXCEPTION_HANDLER);
    }
}

日時

TimeUnit

java.util.concurrent.TimeUnit（列挙型）を使用すると時間の単位変換などをわかりやすく記述できる。

// 4時間をミリ秒単位に変換
System.out.println(TimeUnit.HOURS.toMillis(4));    // -> 14400000
 
// 1日を秒単位に変換
System.out.println(TimeUnit.DAYS.toSeconds(1));    // -> 86400
 
// 3秒スリープ
TimeUnit.SECONDS.sleep(3);

// 1分スリープ
TimeUnit.MINUTES.sleep(1);

Calendarの月の値範囲

java.util.Calendarで扱う月の値範囲は0～11です。（1～12ではありません）
（よく知られた話ですが、分かりにくさがつきまとう。利用時は注意が必要。）

Calendarのメモリ消費量

java.util.Calendarのインスタンスのメモリ消費量は700byteを超えており、普通のクラスに比べてかなりサイズが大きいです。（例えば、java.util.Dateの場合はメモリ消費量 24byte程度です）
そのため、Calendarのインスタンスをクラスのメンバに保持したり、大量に生成したりするのは避けた方が良いです。

JavaBeans

単純なJavaBeansは不整合な可能性あり

単純なgetter/setterを持つJavaBeansはオブジェクトの生成処理が各setterによって分割されているため、生成過程では不整合な状態になっている可能性があるという重大な欠点を持っています。
（特定のメンバには値が入っているが特定のメンバはnullになっていて、そのような状態がクラスとして本当は許容されない等）
クラスを不変（immutable）にすることで基本的には解決します。メンバが多い場合はBuilderパターンを取り入れることでスマートになります。
（※ただし、JavaBeansを必要とする外部ライブラリも存在しますので、そのようなライブラリを使用する場合等はJavaBeansを利用してください）

数値型

double型の正の無限・負の無限・非数

double型には正の無限大値を表すDouble.POSITIVE_INFINITY、負の無限大値を表すDouble.NEGATIVE_INFINITY、非数を表すDouble.NaNが存在します。
通常の処理で有用なのは上記の値を除いた有限値のみです。
外部システムなどの信頼性が確かでないところからdouble値を受け取ったり、文字列からdouble値を作り出す場合などは、上記の値でないことをチェックしておく必要があります。
（非数の挙動として、 Double.NaN == Double.NaN は結果がfalseとなることには注意が必要です）
（正の無限、負の無限、非数についてはfloat型にも同様の事が言えます）

BigDecimalで四捨五入が四捨五入にならない

BigDecimalをdouble型の値を使って生成すると四捨五入処理が場合により五捨六入になってしまいます。
BigDecimalを生成する場合はString型の値を指定するか、BigDecimal.valueOf(double)を使用します。

NG例：

public static BigDecimal roundNumber(double val, int scale) { 
    BigDecimal bd = new BigDecimal(val); 
    bd = bd.setScale(scale, BigDecimal.ROUND_HALF_UP); 
    return bd; 
} 
 
public static void main(String[] args) { 
    System.out.println(roundNumber(9.994, 2).doubleValue()); 
    System.out.println(roundNumber(9.995, 2).doubleValue()); 
    System.out.println(roundNumber(9.996, 2).doubleValue()); 
} 

とすると9.995の四捨五入の結果が10.0ではなく9.99になってしまいます。

防御的プログラミング

防御的コピー

仮にコンストラクタでjava.util.Date型の値を受け取りfinalなメンバとして保持しても、外部から値を変更できてしまいます。
内部的にはDate#getTimeの値を保持しておき必要に応じてDate型に変更するほうが安全です。
このように可変な要素を持つものを保持する場合は防御的にコピーする必要があります。
ただしコピーすることが大きなコストであったり、利用者が同一パッケージで信頼がおける場合はこの限りではありません。
そのような場合は、できればコメントに理由があって防御的コピーを行っていない旨を記載しておくべきです。

■java.util.Dateで外部とやり取りしているクラスの防御有無の各パターンのサンプルコード

●防御的コピーをしていないパターン（コンストラクタもgetterも内部表現を暴露しており、外側からDateHolderの内部表現の日時を変更できてしまう）

public final class DateHolder {
        
    /** 日時 */
    private final Date date;
        
    public DateHolder(Date date) {
        if (date == null) {
            throw new NullPointerException("parameter date is null.");
        }
        this.date = date;
    }
        
    public Date getDate() {
        return date;
    }
}

●防御的コピーをしているパターン（外側からDateHolderの内部表現の日時を変更できない）

public final class DateHolder {
    
    /** 日時（ミリ秒表現） */
    private final long dateMillisec;
    
    public DateHolder(Date date) {
        if (date == null) {
            throw new NullPointerException("parameter date is null.");
        }
        this.dateMillisec = date.getTime();
    }
    
    public Date getDate() {
        return new Date(dateMillisec);
    }
}

■String配列で外部とやり取りしているクラスの防御有無の各パターンのサンプルコード

●防御的コピーをしていないパターン（コンストラクタもgetterも内部表現を暴露しており、外側からStringArrayHolderの内部表現の文字列配列の各要素の値を変更できてしまう）

public final class StringArrayHolder {
    
    /** 文字列の配列 */
    private final String[] strArray;
    
    public StringArrayHolder(String[] strArray) {
        if (strArray == null) {
            throw new NullPointerException("parameter strArray is null.");
        }
        this.strArray = strArray;
    }
    
    public String[] getStrArray() {
        return strArray;
    }
}

●防御的コピーをしているパターン（外側からStringArrayHolderの内部表現の文字列配列の各要素を変更できない）

public final class StringArrayHolder {
    
    /** 文字列の配列 */
    private final String[] strArray;
    
    public StringArrayHolder(String[] strArray) {
        if (strArray == null) {
            throw new NullPointerException("parameter strArray is null.");
        }
        this.strArray = strArray.clone();
    }
    
    public String[] getStrArray() {
        return strArray.clone();
    }
}

名前付け

型を誤解させるような変数名をつけない

以下のような変数宣言があった場合に、

private Map<String, String> addressArray;

この変数を使用しているメソッド等で、"addressArray"という変数名だけ見た場合に対象変数の型は、何かの配列型、ArrayList、独自型あたりだと推測してしまいます。
普通はMap型だとは思わないでしょう。
このように型を誤解させるような変数名はつけないようにした方が良いです。型を誤解させるとコードの可読性が下がります。

配列

配列に意味を持たせない

以下のような（各要素ごとに）意味を持たせた配列を使用すべきではありません。対象の配列を使用しているコードの意味が分かりにくくなりバグ混入の可能性が高まります。
配列の代わりに独自のオブジェクト（クラス）を定義すべきです。

// 氏名と郵便番号、住所を格納する配列
String[] personData = new String[3];
personData[0] = "Taro Yamamoto";    // 氏名
personData[1] = "999-1234";         // 郵便番号
personData[2] = "埼玉県";           // 住所
return personData;

上記の配列の場合、要素数や要素の順番が変わってしまうとそれだけでデータとして不整合な状態となってしまいます。
外部とのインターフェースの都合上どうしても意味を持たせた配列が必要な場面がありますが、そのような箇所は局所的にすべきです。
自システム内に意味を持たせた配列を持ち込みすぎるとコードの可読性が圧倒的に下がります。

コメント

外部に公開するAPIにはJavadocを記述する

外部（別システム）に公開する等で作成者以外が使用することが考えられるAPIについては基本的にjavadocを記載した方が良いです。
対象のクラスがスレッドセーフか、スレッドセーフではないのかを記述してあると利用者側にとってはさらに良いです。
（可能ならば全ての変数、メソッドにjavadocコメントが記述してあるとより良いです）

コードをコメントアウトして残さない

ソースコード内に時々コメントアウトされた処理が存在しますが。コメントアウトを行った人間以外から見て、そのコメントアウト部分のコードはノイズにしかなりません。
基本的に不要となった処理はコード上に残さず、即時に削除すべきです。（過去に戻りたい場合はバージョン管理システムの出番です！）
どうしても何らかの理由があって処理コードをコメントアウトして残す場合は、どういう理由でコメントアウトしてあえて残しているのかを強くコメントに記述（補足）しておくべきです。
ただし、残しておくべき理由がなくなったらコメントアウトしたコードは削除すべきです。
（本件はJavaに限らずバージョン管理されている全プログラムにおいて言えることです）

JUnit

テストし易いコードを書く

• ユニットテストの対象クラスやメソッドが、テストしにくければ、当然ですがJUnitでテストケースを書くのは困難になります。
  テストし易いコードを書くように心掛けた方が良いです。
  テストしにくいということは、実際に（対象のメソッドやクラスを）利用する側から見ても扱いにくいメソッド（やクラス）であると言えます
• テストし易いメソッドとは？
  • 引数を取る
  • 引数の数が少ない
  • メソッド内の処理中に引数を変更しない
  • 戻り値がある
  • 同一の引数に対して常に同一の戻り値を返す
  • そのメソッドを提供するクラスのインスタンス化がし易い
  • 外部環境（外部システム）に依存していない

Mockライブラリ

ユニットテストを書く際に、Mock化したインスタンスを生成するライブラリを使用すると、ユニットテストケース作成の難易度が下がる場合もあります。
代表的なMockライブラリは以下になります。

• Mockito
• JMockit

適切なAPIを使用する

実行効率の悪いコンストラクタ

以下に示すコンストラクタは非効率であるため使用すべきではありません
代替案を使用すべきです。

• new String()
  • 代替案： ""（空文字列）を使用すべきです
• new String(String)
  • 代替案： 引数にとっている文字列をそのまま使用すべきです
• new Integer(int)
  • 代替案： Integer.valueOf(int) を使用すべきです（その他のプリミティブ型でも同様）
• new Boolean(...)
  • 代替案： Boolean.valueOf(...) を使用すべきです

初期化

staticイニシャライザに複雑な処理を書かない

staticイニシャライザ（静的初期化子）でネストが深くステップ数の多い処理を行うのはNGです。
staticイニシャライザはメソッド等とは違い引数も戻り値もないため、複雑なロジックを書いてもユニットテストでの確認が困難となります。
テスト容易性を確保する上でもstaticイニシャライザに複雑な処理を記述すべきではありません。

インスタンスイニシャライザを使用しない

インスタンスイニシャライザは存在があまり知られていないインスタンスの初期化方法のため基本的に使用すべきではありません。
インスタンスの初期化処理は、コンストラクタに記述すべきです。インスタンスイニシャライザを使用するとコードの可読性が落ちてしまいます。
（ただし、無名クラスにおける初期化処理においては有効に利用できます。が、使用するとしてもその用途に限るべきです。）

クラス構成

ユーティリティクラス

Javaでユーティリティクラスの様なstaticメソッドのみを持つクラスのソースコードの典型例は以下の通りです。

public final class SampleUtils {

    public static void func1() {
        // do something
    }
    
    public static void func2() {
        // do something
    }

    /**
     * インスタンス生成の抑制
     */
    private SampleUtils() {
    }
}

上記コードのポイントは以下２つです。

• クラスをfinalにして継承による拡張を防ぐ（staticなクラスなので継承は想定されない）
• コンストラクタをprivateにしてインスタンス化を抑制する（インスタンス化する意味が無いため）

実装クラスは基本的にfinalに

abstractでない（継承されることを想定しない）実装クラスには基本的にfinal修飾子を付与して下さい。
特に他システム等の外部と連携するために公開している実装クラスをfinalにしていない場合、思いもよらない継承をされてしまう場合があります。
想定外のサブクラスが存在してしまうと継承元のクラスが簡単に修正できなくなってしまいます。

パッケージプライベートによるクラスの隠蔽

以下のように実装クラスの可視性をパッケージプライベートにすることで、外部に対して実装を隠蔽することができます。
（※本内容はどんな場面でも使用できるわけではないですがJavaの道具立ての一つとして理解しておくことで、クラス構成・パッケージ構成を考える際の役に立つと思います）

外部に公開しているプロジェクトのパッケージ及びクラス構成は以下の通りとする。
（org.sample.somepackage.apiパッケージに３つファイルがあるとする）

org.sample
└─somepackage
    └─api
         SomethingInterface.java
         SomethingImpl.java
         SomethingFactory.java

各ファイルの概要及び可視性は以下の通り

ファイル	可視性	概要
SomethingInterface.java	public	外部が使用することを想定したインターフェース
SomethingImpl.java	package private	SomethingInterfaceの実装クラス
SomethingFactory.java	public	SomethingInterfaceの実装を生成して返却するファクトリー

この構成の主な利点は以下２つです。

• ①外部（他システム、他パッケージ）からインターフェースに対する実装が隠蔽される
  • 外部からSomethingInterfaceを使用したい場合、SomethingFactoryを経由してしか（実装が）取得できない
  • 外部からはSomethingImplの存在が全く見えない
• ②実装クラスのユニットテストはできる
  • SomethingImplは外部からは操作できないが同一パッケージからは見えるので、同一パッケージ上に配置するテストケースクラスからは直接操作（new）してテストすることができる。

Immutableなクラスにすることを考える

Immutableなクラスはマルチスレッド環境でも同期処理等無しで正常に動作します。クラスをImmutableで設計することは重要です。
Immutableなクラスの例は以下の通りです。
メンバがfinalであるため、コンストラクタ実行後に変更することができません。一般的なJavaBeansと違ってsetterを持ちません。

public final class ImmutableObject {

    private final int id;

    private final String value;

    public ImmutableObject(int id, Sring value) {
        this.id = id;
        this.value = value;
    }

    public int getId() {
        return id;
    }
    
    public String getValue() {
        return value;
    }
}

メソッド間の値の受け渡し用インスタンス変数を作らない

以下のサンプルコードではexecuteMainLogicメソッドからexecuteLogicDetailをメソッドを呼び出す際の値の受け渡しにインスタンス変数tempVariableを利用しています。
このように、インスタンス変数をメソッドの値の受け渡しのために使用するのはNGです。これを行うと対象クラスはスレッドセーフではなくなります。
下記の例で言えばメソッド間の値の受け渡しには素直に引数を使用すべきです。

public class TempVariableTrial {

    /** executeMainLogic -> executeLogicDetail へ値を渡すための変数 */
    private int tempVariable = 0;
    
    public void executeMainLogic(int value) {
        
        if (value < 0) {
            tempVariable = -1;
        } else {
            tempVariable = 1;
        }
        
        executeLogicDetail();
    }

    private void executeLogicDetail() {
        if (tempVariable > 0) {
            // do something tempVariable positive number
        } else {
            // do something tempVariable zero or negative number
        }
    }
}

Nullオブジェクト

処理中にnullチェック、nullによる分岐が頻発している場合はNullオブジェクトの導入を検討した方が良いです。
処理中にnullを元にした処理が多いとバグが混入しやすくなったり、コードの可読性がおちます

外部に公開するAPI

Javaで外部に公開して利用してもらう手続き(API)の設計方針は以下の通りです。
（実際には外部に公開しないものであっても、以下を意識しておくことは重要です）

• 手続きの名称が適切で、それを利用したコードの可読性が高いこと。
• 利用者への要求が可能な限り少ないこと。（引数が少ない、API利用までのセットアップ等が少ない（無い））
• 基本的には実装でやりとりせずインターフェースで会話させる。（引数、戻り値も可能ならインターフェースにする）
• 引数、戻り値で受け渡すオブジェクトは極力Immutableにする。（不用意な破壊を防ぐ、スレッドセーフ性の確保）
• スレッドセーフ性を確保する。（利用側にスレッドセーフにするための同期処理をさせなくて済むようにする）
• 副作用を起こさない。（手続きの名前から想像できない外部環境へ影響のある処理をしない）
• 公開する手続きがチェック例外をスローすると利用側にとって負担になることを理解しておく。

staticメソッドだけを持つクラスの注意点

スレッドセーフ性を意識したクラス設計をしていると全くインスタンス変数を持たない（インスタンスメソッドだけを持つ）クラスになる場合があります。
そういった場合に、都度インスタンスを利用側が生成するのも面倒だろうから、メソッドを思いっ切ってstaticメソッドに置き換えてしまっても良いのではないかという考えに陥る場合があります。
しかし、両者には決定的な違いがあることに注意が必要です。
staticメソッドだけにしてしまうとポリモーフィズムを全く利用できなくなるという点です。そうなると必ず実装を利用側に見せる必要がでてきます。
汎用的なユーティリティクラスをstaticメソッドのみのクラスとして実装するのはOKですが、外部に公開するような業務的なAPIをstaticにするのは避けた方が得策です。

クラスはできるだけ小さく

クラスのステップ数が肥大化するとプログラマの手に負えなくなってしまいます。できるだけクラスのステップ数を小さくするように努めるべきです。
ステップ数が肥大化したクラスに対して機能追加などのために変更が必要な場合、変更の影響範囲を的確に検出するための簡単な方法が無いため、
「編集して、そして祈る」というプログラミング手法になってしまいます。
そうなると変更にとても長い時間を要するか、バグが増加するかのどちらかになります。

生成処理

コンストラクタの隠蔽を検討する

実装クラスのコンストラクタを直接呼ばれるとクラス間の結合度が高くなってしまいます。
また、コンストラクタはコード上 "new" としか書けないので、どういうオブジェクトを生成するのか という意味を持たせるのが難しくなります。
全ての場面においてコンストラクタを見せないようにする必要はありませんが、外部に見せる実装クラスにおいてはコンストラクタを隠蔽する検討をした方がよいでしょう。
例えば、コンストラクタの代わりにstaticファクトリを提供することで、どういうオブジェクトを生成しようとしているかの意味が分かりやすくなったり、
生成処理を隠蔽することで、毎回新しいインスタンスを返すのではなく（キャッシュのように）１つのインスタンスを使いまわして返すことがやり易くなります。

■コンストラクタを公開（public）にして、利用側に毎回コンストラクタを実行してもらうパターン

/**
 * 処理結果のメッセージを表すオブジェクト（コンストラクタ公開パターン）
 */
public final class ResultMsg {

    /** 成功を意味するメッセージか？ */
    private final boolean isSuccess;
    
    /** メッセージ文字列 */
    private final String message;

    /**
     * 結果メッセージを生成します。
     * 
     * @param isSuccess 成功を意味するメッセージの場合true, それ以外の場合false
     * @param message メッセージ文字列
     */
    public ResultMsg(boolean isSuccess, String message) {
        this.isSuccess = isSuccess;
        this.message = message;
    }

    /**
     * 成功を意味するメッセージか？を取得します。
     * @return 成功を意味するメッセージか？
     */
    public boolean isSuccess() {
        return isSuccess;
    }

    /**
     * メッセージ文字列を取得します。
     * @return メッセージ文字列
     */
    public String getMsg() {
        return message;
    }
    
    
    /**
     * 対象クラスを実行してみるためのサンプルmain
     * 
     * @param args 実行時の引数
     */
    public static void main(String[] args) {
        // コンストラクタを実行してオブジェクトを生成する
        ResultMsg okMsg = new ResultMsg(true, "The operation was successful.");     // 成功メッセージ
        ResultMsg ngMsg = new ResultMsg(false, "The operation was failed.");        // 失敗メッセージ
        ResultMsg emptyMsg = new ResultMsg(true, "");                               // 空メッセージ
    }
}

■コンストラクタを非公開（private）にして、利用側にstaticファクトリを実行してもらうパターン
（呼び出し側コードにおいて、成功 or 失敗のメッセージを生成していることが分かりやすくなる）

/**
 * 処理結果のメッセージを表すオブジェクト（コンストラクタ非公開パターン）
 */
public final class ResultMsg {
    
    /** 空を意味するメッセージ */
    private static final ResultMsg EMPTY_MSG = ok("");

    /** 成功を意味するメッセージか？ */
    private final boolean isSuccess;
    
    /** メッセージ文字列 */
    private final String message;
    
    /**
     * 成功メッセージを生成します。
     * 
     * @param message メッセージ文字列
     * @return 成功メッセージを意味するオブジェクト
     */
    public static ResultMsg ok(String message) {
        return new ResultMsg(true, message);
    }
    
    /**
     * 失敗メッセージを生成します。
     * 
     * @param message メッセージ文字列
     * @return 失敗メッセージを意味するオブジェクト
     */
    public static ResultMsg ng(String message) {
        return new ResultMsg(false, message);
    }

    /**
     * 空を意味するメッセージを返します。
     * 
     * @return 空のメッセージを意味するオブジェクト
     */
    public static ResultMsg empty() {
        return EMPTY_MSG;
    }

    /**
     * 結果メッセージを生成します。
     * 
     * @param isSuccess 成功を意味するメッセージの場合true, それ以外の場合false
     * @param message メッセージ文字列
     */
    private ResultMsg(boolean isSuccess, String message) {        
        this.isSuccess = isSuccess;
        this.message = message;
    }

    /**
     * 成功を意味するメッセージか？を取得します。
     * @return 成功を意味するメッセージか？
     */
    public boolean isSuccess() {
        return isSuccess;
    }

    /**
     * メッセージ文字列を取得します。
     * @return メッセージ文字列
     */
    public String getMsg() {
        return message;
    }
    
    /**
     * 対象クラスを実行してみるためのサンプルmain
     * 
     * @param args 実行時の引数
     */
    public static void main(String[] args) {
        // staticファクトリー経由でオブジェクトを生成する
        ResultMsg okMsg = ResultMsg.ok("The operation was successful.");   // 成功メッセージ
        ResultMsg ngMsg = ResultMsg.ng("The operation was failed.");       // 失敗メッセージ
        ResultMsg emptyMsg = ResultMsg.empty();                            // 空メッセージ
    }
}

ロック

Semaphoreをバイナリセマフォとして利用する際の注意

java.util.concurrent.Semaphoreをバイナリセマフォ（いわゆるmutex）のように使用する場合は注意が必要です。
コンストラクタで指定したpermitsの初期値を1とした場合でも、releaseメソッドを呼び出す度に値が初期値を無視して2, 3, 4と増えます。
そのため、思いもよらないタイミングでtryAcquireが間違って成功してしまう場合があります。
（バイナリセマフォとして利用するには内部のpermitsが必ず0か1でのみ遷移するように外部から制御するなどの処置が必要になります）

リフレクション

リフレクションの利用は最低限にする

普通に書いたプログラムでは実現できないことが、リフレクションを使うと実現できたりします。
ただし、リフレクションを使いすぎると処理が分かりにくくなったり、速度的な問題などが出る可能性もあります。
普通のアプリケーションにおいては、リフレクションはあまり利用しないのが得策です。
使う場合も限定的であった方が良いです（フレームワークや特定のライブラリを実装する側etc）

セキュリティ

ライブラリの脆弱性

利用しているライブラリに時間経過とともにセキュリティ的な問題が見つかる場合があります。
定期的にOWASP Dependency Check等で脆弱性が存在していないかチェックした方が良いです

補足

長期的にシステムを育てていくためには、ここに書いた点以外でもプログラムの可読性やメンテナンス容易性の点での考慮も必要です。その点について以下エントリに書いています。

また、Javaでプログラムを書くという点とは少し異なりますが、システムを俯瞰してみた場合に考慮しておいた方がよい内容を以下に書いています