仕事でSpring Bootを触る機会が増えてきました。Springは，アノテーションや関連機能を駆使すれば，かなり高い生産性でアプリを作れる反面”何となく”で使ってしまうと意図しない挙動を招く部分も多々あります。今回はコードレビュー時にコメントすることが多い内容(初歩的なポイントばかりですが……)を整理してみようと思います。
(1) Controller/Service/Repositoryに分割しているか？
Springではアプリケーション全体の処理を特定のクラスにべた書きするのではなく，以下の3層構造クラスに分割して実装することを推奨しています。
| 層 | 概要 |
|---|---|
| Controller | リクエストの窓口になるクラス |
| Service | ビジネスロジックを実装するクラス |
| Repository | データ永続化を行うクラス |
リクエストの入り口になるのはControllerになるため，初めてSpringを触る開発者の方も馴染みに深いと思うのですが，何も考えずにアプリを作っていくと，ついついControllerにビジネスロジックを実装してしまいがちです。そうすると，所謂保守性や可読性が低いFat Controllerが出来上がってしまうので，ビジネスロジックの隔離は強く意識したいポイントだと思います。
個人的にはそれぞれ以下の内容に徹するのが良いのでは？という考えです。
- Controller
- リクエストの受付
- リクエストパラメータのValidation
- Serviceの実行
- 例外処理
- レスポンスの作成
- Service
- ビジネスロジックの実行
- 外部サービスとの連携
- Repositoryを介したデータ操作
- トランザクション管理
- Repository
- データ操作
Controllerの中で複数種類のServiceを呼び出す場合は，Controllerが大きくなりがちなので，その場合はControllerとServiceを仲介するServiceFacadeクラスを間に作っても良い気がします。
(2) インスタンスのライフサイクルを考慮した実装か？
SpringのDIコンテナで管理されるインスタンスは，デフォルトではSingletonになります。つまり，メンバ変数などで状態を持ってしまうと，同じアプリケーションにアクセスしているユーザ間で状態(ユーザID等)が混じってしまい，意図した挙動にならないことが多いです。基本的にはStatelessに作った方が良いと思いますが，要件次第では，Springがサポートしている他のスコープに変更することも検討してください。
| スコープ | 範囲 |
|---|---|
| Singleton | アプリケーションで1つ |
| session | セッションごとに1つ |
| prototype | アクセスごとに１つ |
ちなみに自前クラスであれば@Scopeアノテーションを追加することでスコープを変更可能です。
@Scope("prototype")
@Service
public class SecretService {
//処理実装
}
チェック観点としては
- Singletonなインスタンスで状態を持っていないか？
- 不必要に他のスコープ(session/prototype)を使っていないか？
あたりがポイントになってきます。
(3) コンストラクタ インジェクションを使っているか？
Springを使っていると避けて通れない依存性注入(DI)ですが，設定ファイルを書かない方式だと以下の2種類の方法があります。※おそらくXMLでの定義もまだ生きてると思いますが，使っている人が少ないと思うので割愛。
- フィールドインジェクション
- コンストラクタインジェクション
フイールドインジェクションはメンバ変数に@Autowiredを付けるだけなので簡単です。
@Autowired
private SecretService service;
しかし，以下に挙げる欠点もあるため，非推奨になっています。※Springでも警告を出してくれた記憶があります。
- テストコード作成時にDIコンテナを使わないとモックが出来ない
- フィールドにfinal属性を付与出来ずimmutableにならない
そのため，よほどの事情(どうしても循環依存が要るとか)がない限りは，コンストラクタインジェクションを使いましょう。
private final SecretService service
public MyClass(SecretService service) {
this.service = service;
}
コンストラクタの定義が煩雑なのであれば，Lombokを活用するのも手です。
@AllArgsConstructor
public class MyClass {
//TODO: 実装
}
(4) 共通化できる処理がないか？
Springでは，横断的な共通処理を定義する仕組みとしてHandlerInterceptorやAOPを提供しています。これらの仕組みを活用することで
- 特定パッケージ配下にある全てのクラスの全メソッドで共通の前処理を行う
- 特定の型の戻り値を返すメソッドで共通の後処理を行う
なんて場合に重複した処理を実装しなくてもよくなります。よくあるケースとしてログ出力あたりが多いでしょうか。
/**
* ロガークラス
*/
private final Logger logger = LoggerFactory.getLogger(MyController.class);
/**
* リクエストメソッドのサンプル.
* @param form Formクラス
* @param bindingResult Validation結果
* @return レスポンス
*/
@RequestMapping(value = "/", method = RequestMethod.GET)
public String handleRequest(@Valid MyControllerForm form, BindingResult bindingResult) {
logger.info("Interceptorテスト");
//省略
}
}
具体的に
@Aspect
@Component
public class MyInterceptor {
@Before("execution(* jp.co.cross_xross.controller..*.*(..))")
public void beforeProcess(JoinPoint joinPoint) throws Throwable {
//前処理
}
}
のようにパッケージ名やメソッド名，引数，戻り値を指定して「前処理」「後処理」「代替処理」を実行可能です。
(5) Springが提供している機能を自前実装していないか？
そろそろネタが枯渇してきました(笑) Springでは，よく実装しがちな機能をサブプロジェクトとして提供しています。自前実装で苦労しがちな部分も当該機能を使えば容易に実現できるので活用した方が良いと思います。
| サブプロジェクト名 | 提供機能 | 備考 |
|---|---|---|
| Spring Session | セッションレプリケーション | - |
| Spring Security | 認証機能 | - |
| Spring Data | データ(RDBMS/KVS)操作 | - |
| Spring Social | SNS(Facebook/Twitter/LinkedIn)連携 | その他SNSとの連携も可能 |
| Spring Batch | バッチ処理 | - |
| Spring AMQP | RabbitMQ連携 | - |
あとがき......
Javaの基本的なコードレビューポイントと併せて確認しておきたいところです。ピュアJavaのコードレビュー観点としては，過去にqiitaで書かれた諸先輩方の記事を参考にしています。