0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

@doc_lab

未経験の時に書いたJavaソースをリファクタリングしてみた

Posted at

はじめに

未経験の時に作成したJavaのソースコードをGitHubで見つけたので、実務で2年間学んだ知識を活かしてリファクタリングしてみました。
これまではWebアプリケーションからサーバーレスシステムまで幅広く触れてきましたが、改めてJavaの「設計」に深く向き合ってみると、意外な発見があり楽しかったです。

※全容は公開できないため、主要なロジックのみを抜粋して解説します。

リファクタリング前のソース

MVCモデルを使ってきちんとオブジェクト指向で書かれていますが、Controllerに色々な処理が混在している状態です。
ビジネスロジックも直接書かれていてJUnit等を使用した単体テストができません。

CandidateRegiCon.java
package controller;

import java.io.IOException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Date;

import javax.servlet.RequestDispatcher;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import model.InfoBean;
import model.InfoDao;
import model.SecBean;
import util.FileUploadOriginal;
import util.Mail;

/**
 * Servlet implementation class CandidateRegiCon
 */
public class CandidateRegiCon extends HttpServlet {
	private static final long serialVersionUID = 1L;

    /**
     * @see HttpServlet#HttpServlet()
     */
    public CandidateRegiCon() {
        super();
        // TODO Auto-generated constructor stub
    }

	/**
	 * @see HttpServlet#doGet(HttpServletRequest request, HttpServletResponse response)
	 */
	protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
		// 文字コードの指定
		request.setCharacterEncoding("UTF-8");

		//日付クラスの書式を設定したSimpleDateFormatクラスのインスタンスを作成する
		SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd");
		Date now = new Date();

		// InfoDaoクラスのインスタンスを生成
		InfoDao dao = new InfoDao();
		// 部署情報検索メソッド（一覧表示画面）の呼び出し
		ArrayList<SecBean> allSectionList = dao.allSectionSearchDao();

		// JSPへ渡す
		request.setAttribute("allSectionList", allSectionList);
		request.setAttribute("today",sdf.format(now));

		// JSPファイルを表示（応募者登録画面）
		ServletContext app = this.getServletContext();
		RequestDispatcher dispatcher = app.getRequestDispatcher("/CandidateRegi.jsp");
		dispatcher.forward(request,response);
	}

	/**
	 * @see HttpServlet#doPost(HttpServletRequest request, HttpServletResponse response)
	 */
	protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
		// 文字コードの指定
		request.setCharacterEncoding("UTF-8");

		// InfoDaoのインスタンスを生成
		InfoDao dao = new InfoDao();
		// InfoBeanクラスのインスタンスを生成
		InfoBean infoBean = new InfoBean();
		// FileUploadOriginalクラスのインスタンスを生成
		FileUploadOriginal file = new FileUploadOriginal();

		// ファイルをアップロードするパスの取得
		String path = getServletContext().getRealPath("files");
		// ファイルのアップロードメソッドの呼び出し（その他のデータはInfoBeanに格納）
		infoBean = file.FileUp(request,path);
		// ファイル名をfileName変数へ代入
		String fileName = infoBean.getCandiPdf();
		// 拡張子チェック
		boolean extension = false;
		// ファイル名が存在するか確認
		if(fileName != null) {
			extension = true;		// 存在するので拡張子チェックOK
		}else {
			extension = false;		// 存在しないので拡張子チェックNG
		}

		// 拡張子が対応しているかどうか
		if(extension == false) {
			// JSPへ渡す
			request.setAttribute("extension", extension);

		}else {
			// 応募者名重複チェックメソッドの呼び出し・結果の取得
			//int count = dao.registerCheckDao(infoBean);
			// 同姓同名対策
			int count = 0;
			// 応募者登録の有無を設定する変数
			int num = 0;

			//取得した重複件数によって処理分け
			if( count == 0 ) {
				// 応募者登録メソッドの呼び出し・結果の取得
				num = dao.registerDao(infoBean);
				// メールアドレス取得メソッドの呼び出し
				ArrayList<String> mailaddress = dao.getMailaddressDao(infoBean.getSecName());
				// Mailクラスのインスタンスを生成
				Mail mail = new Mail();
				// 応募者の希望部署に所属するユーザに対するメール送信（複数対応可）
				for(int i = 0; i < mailaddress.size(); i++) {
					mail.sendMail(mailaddress.get(i));
				}
			}

			// JSPへ渡す
			request.setAttribute("result", num);
		}
		// 部署情報検索メソッド（一覧表示画面）の呼び出し
		ArrayList<SecBean> allSectionList = dao.allSectionSearchDao();

		// JSPへ渡す
		request.setAttribute("allSectionList", allSectionList);
		request.setAttribute("infoBean", infoBean);
		request.setAttribute("secName", infoBean.getSecName());

		// JSPファイルを表示（応募者登録画面）
		ServletContext app = this.getServletContext();
		RequestDispatcher dispatcher = app.getRequestDispatcher("/CandidateRegi.jsp");
		dispatcher.forward(request,response);
	}

}

リファクタリング後の同じソース

ビジネスロジックをサービス層に書くことによってだいぶスッキリしました。
また、メール通知機能は拡張性を持たせてインターフェースを使用しています。
今後、LINE通知等が追加になってもController側の修正は不要になります。（または少量の修正で済む）

①. forwardのような共通処理を切り出す
②. 「/xxx.jsp」のようなパスは①と同様に共通処理の中に定数として切り出す
③. サービス層にビジネスロジックを移動し、Controllerでは基本的に呼び出しのみ行う。

CandidateRegiCon.java
package controller;

import java.io.IOException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Date;

import javax.servlet.RequestDispatcher;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.Part

import model.InfoBean;
import model.SecBean;

import service.CandidateService;
import service.SectionService;
import service.NotificationService;
import service.MailNotificationService;
import util.CommonUtils;

/**
 * Servlet implementation class CandidateRegiCon
 */
public class CandidateRegiCon extends HttpServlet {
	private static final long serialVersionUID = 1L;

    /**
     * @see HttpServlet#HttpServlet()
     */
    public CandidateRegiCon() {
        super();
        // TODO Auto-generated constructor stub
    }

	/**
	 * @see HttpServlet#doGet(HttpServletRequest request, HttpServletResponse response)
	 */
	protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
		// 文字コードの指定
		request.setCharacterEncoding("UTF-8");
		try{
			//日付クラスの書式を設定したSimpleDateFormatクラスのインスタンスを作成する
			SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd");
			Date now = new Date();

			// 部署一覧情報を全て取得
			SectionService sectionService = new SectionService(new InfoDao());
			ArrayList<SecBean> allSectionList = sectionService.getAllSectionList();

			// JSPへ渡す
			request.setAttribute("allSectionList", allSectionList);
			request.setAttribute("today",sdf.format(now));

			// JSPへフォワード（応募者登録画面）
			CommonUtils.forwardTo(request, response, CommonUtils.CANDIDATE_REGI_JSP);
		}catch (Exception e){
			// エラーログの出力（開発者用）
			e.printStackTrace();
			
			// ユーザーへの通知
	        request.setAttribute("errorMessage", "システムエラーが発生しました。管理者へ連絡してください。");
	        // エラー専用のJSP、または元の入力画面にフォワード
	        CommonUtils.forwardTo(request, response, CommonUtils.ERROR_JSP);
		}
	}

	/**
	 * @see HttpServlet#doPost(HttpServletRequest request, HttpServletResponse response)
	 */
    @MultipartConfig
	protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
		// 文字コードの指定
		request.setCharacterEncoding("UTF-8");

		// 応募者情報のサービスのインスタンスを生成
		CandidateService candidateService = new CandidateService(new InfoDao(), new FileUploadOriginal());
		
		try {
            // リクエストからファイルを取り出す
            Part filePart = request.getPart("candiPdf");
			// ファイルをアップロードするパスの取得
			String path = getServletContext().getRealPath("files");
			// ファイルアップロード実行
			InfoBean infoBean = candidateService.uploadFile(filePart,path);

			// 拡張子が対応しているかどうか（拡張子チェック）
			if(candidateService.isValidExtension(infoBean)) {
				// 応募者名重複チェックメソッドの呼び出し・結果の取得
				//int count = candidateService.checkDuplicates();
				// 同姓同名対策
				int count = 0;
				// 応募者登録の有無を設定する変数
				int num = 0;

				//取得した重複件数によって処理分け
				if( count == 0 ) {
					// 応募者登録メソッドの呼び出し・結果の取得
					num = candidateService.registerCandidate(infoBean);
					// メールアドレス取得メソッドの呼び出し
					ArrayList<String> mailaddressList = candidateService.getMailAddress(infoBean);
					//関係部署メンバーへメール送信
					NotificationService notificationService = new MailNotificationService();
					notificationService.sendNotification(mailaddressList);
				}

				// JSPへ結果を渡す
				request.setAttribute("result", num);
			}else {
				// 対応していない拡張子が含まれているのでJSPへfalseを渡す
				request.setAttribute("extension", false);
			}
			
			// 部署一覧情報を全て取得
			SectionService sectionService = new SectionService(new InfoDao());
			ArrayList<SecBean> allSectionList = sectionService.getAllSectionList();

			// JSPへ渡す
			request.setAttribute("allSectionList", allSectionList);
			request.setAttribute("infoBean", infoBean);
			request.setAttribute("secName", infoBean.getSecName());

			// JSPへフォワード（応募者登録画面）
			CommonUtils.forwardTo(request, response, CommonUtils.CANDIDATE_REGI_JSP);
		}catch (Exception e){
			// エラーログの出力（開発者用）
			e.printStackTrace();
			
			// ユーザーへの通知
	        request.setAttribute("errorMessage", "システムエラーが発生しました。管理者へ連絡してください。");
	        // エラー専用のJSP、または元の入力画面にフォワード
	        CommonUtils.forwardTo(request, response, CommonUtils.ERROR_JSP);
		}
	}

}

サービス層

ここにビジネスロジックを集約しました。
コンストラクタに引数を渡す事で単体テストをしやすい設計になっています。
※依存性の注入（Dependency Injection / DI）
Mockオブジェクト（Mockitoなど）を差し込みやすくなりました。
できるだけ疎結合になるようにしました。

CandidateService.java
package service;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.Part
import model.InfoBean;
import model.InfoDao;
import util.FileUploadOriginal;
import java.util.ArrayList;

//応募者情報のサービス
public class CandidateService {
    private final InfoDao dao;
    private final FileUploadOriginal fileUpload;

    // コンストラクタで変数初期化
    public CandidateService(InfoDao infoDao, FileUploadOriginal fileUpload) {
        this.dao = infoDao;
        this.fileUpload = fileUpload;
    }

    /**
     * 応募者のファイルアップロード操作を行う
     * @param filePart リクエストから抽出されたファイルデータ（input type="file" の内容）
     * @param uploadPath ファイルを保存する物理パス
     * @return infoBeanクラス
     */
    public InfoBean uploadFile(Part filePart, String uploadPath) {
        // ファイルアップロード処理の実行
        InfoBean infoBean = fileUpload.FileUp(filePart, uploadPath);
        return infoBean;
    }
    /**
     * 応募者の重複確認を行う
     * @return ヒット件数
     */
    public int checkDuplicates(InfoBean infoBean){
    	return dao.registerCheckDao(infoBean);
    }
    /**
     * 応募者の登録処理を行う
     * @return 登録件数
     */
    public int registerCandidate(InfoBean infoBean){
    	return dao.registerDao(infoBean);
    }
    /**
     * 応募者のメールアドレスを取得する
     * @return メールアドレス（String）が格納されたArrayList
     */
    public ArrayList<String> getMailAddress(InfoBean infoBean){
    	return dao.getMailaddressDao(infoBean.getSecName());
    }
    /**
     * ファイルの拡張子が対応しているか確認
     * @return boolean true or false
     */
    public boolean isValidExtension(InfoBean infoBean){
    	boolean extension = false;
    	String fileName = infoBean.getCandiPdf();
    	// ファイル名が存在するか確認
		if(fileName != null) {
			extension = true;		// 存在するので拡張子チェックOK
		}else {
			extension = false;		// 存在しないので拡張子チェックNG
		}
		return extension;
    }

}
SectionService.java
package service;

import java.util.ArrayList;
import model.InfoDao;
import model.SecBean;

//部署情報のサービス
public class SectionService{
	private final InfoDao dao;

	// コンストラクタで変数初期化
	public SectionService(InfoDao infoDao) {
        this.dao = infoDao;
    }
    
    /**
	 * 部署の一覧情報をデータベースから全件取得
	 * @return 部署情報（SecBean）が格納されたArrayList
	 */
    public ArrayList<SecBean> getAllSectionList(){
    	return dao.allSectionSearchDao();
    }
}
NotificationService.java
package service;
import java.util.ArrayList;

public interface NotificationService{
	void sendNotification(ArrayList<String> mailaddressList);
}
MailNotificationService.java
package service;
import java.util.ArrayList;
import util.Mail;

//メール通知のサービス
public class MailNotificationService implements NotificationService{
	private final Mail mail;
	
	public MailNotificationService(){
		this.mail = new Mail();
	}
	/**
     * 指定されたメールアドレスのリストに対して、通知メールを一括送信
     * 応募者の希望部署に所属するユーザー全員に送信することを想定
     * @param mailaddressList 送信先メールアドレスのリスト
     */
    @Override
	public void sendNotification(ArrayList<String> mailaddressList){
		// 応募者の希望部署に所属するユーザに対するメール送信（複数対応可）
		for(int i = 0; i < mailaddressList.size(); i++) {
			mail.sendMail(mailaddressList.get(i));
		}
	}
}

共通系

実務でもよく「定数は直接書かないで切り出して」と言われます。
CommonUtils.javaのようにパスを定数にすることにより、直接書く必要がなくなり保守性が高まります。
もしController内に直接「/CandidateRegi.jsp」と毎回書いていたら、パスが変わった時に全部直さないといけなくなってしまいます。
定数にする事により、この変数の値だけを変更すると全てに反映される仕組みです。
個人的には意外とやりがちな事だと思っています。

CommonUtils.java
package util;

import java.io.IOException;
import javax.servlet.RequestDispatcher;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

//共通処理クラス
public class CommonUtils {
	/** JSPパス */
	public static final String CANDIDATE_REGI_JSP = "/CandidateRegi.jsp";
	public static final String ERROR_JSP = "/error.jsp";
	/**
	 * 指定されたURLにリクエストをフォワードします。
	 * @param forwardURL 転送先の相対パス
	 * @throws ServletException 転送処理中にサーブレット例外が発生した場合
	 * @throws IOException 入出力エラーが発生した場合
	 */
	public static void forwardTo(HttpServletRequest request, HttpServletResponse response, String forwardURL)throws ServletException, IOException {
        ServletContext app = request.getServletContext();
        RequestDispatcher dispatcher = app.getRequestDispatcher(forwardURL);
        dispatcher.forward(request, response);
}

おわりに

就職する前はプログラミングスクールでしっかりJavaを勉強したつもりでいました。
実務で経験を積んでも意外と難しいと思う箇所があり時間がかかりました。
私の携わっているwebアプリケーションは既に歴史が長く、ブラックボックス化されている部分も多いので開発はとても楽です。
その代わり、今回やってみたリファクタリングに必要な考え方が詰まっているロジックはとても見えずらくなっています。

開発のスピード感を上げる為にはブラックボックス化がとても重宝しますが、デメリットとして私のような未熟な開発者にとって設計の経験は積みづらいと思いました。

実際にリファクタリングをやってみて技術力の向上につながりました！

0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?