java - この Java メソッドをリファクタリングして OOP をより有効に活用する方法

Question

このコードをリファクタリングしてよりクリーンにし、より良い OOP プラクティスを使用しようとしています。このメソッドは、多数のラジオ/チェックボックスとテキストボックスの応答を受け取り、データベーステーブルでそれらを更新してから、別のデータベーステーブルでチェックリスト自体を更新します。

この方法は多くのことをしようとしているように感じます。しかし、他のメソッドやクラスが使用するいくつかのことを決定する必要があります。たとえば、欠陥が存在するかどうか (無線値 = 2)、ワークフローを進めるかどうか (processUpdateCheckbox で決定されるadvanceWorkflow ブール値)、ステータスに基づいて次に誰に電子メールを送信するかなどです。 currentActionItem と AdvanceWorkflow ブール値、および応答を永続化します。

メソッドがフォームデータを処理している別のサーブレットから呼び出され、このメッセージが失われるため、setFormFeedback もここには属しません。

これをリファクタリングする際の助けは大歓迎です。

public ChecklistInstance updateYesNoNAChecklistTogles(HttpServletRequest request, ChecklistInstance ci) throws DAOException {
    String work_item_class_id = request.getParameter("work_item_class_id");
    String work_action_class_id = request.getParameter("work_action_class_id");

    String paramName;
    String attribute_id;
    String radioValue;
    String textValue;
    String strStatus = "1";
    String strStoredNo = "";
    Date dateNow = new Date();

    YesNoNAAnswerDAO ynnDao = new YesNoNAAnswerDAO();
    ChecklistInstanceDAO ciDao = new ChecklistInstanceDAO();

    WorkflowInstanceWorkItemAction currentActionItem = new WorkflowInstanceWorkItemAction();
    currentActionItem.setWork_item_class_id(work_item_class_id);
    currentActionItem.setWork_action_class_id(work_action_class_id);

// Put the form check list responses into a list
    List answer_attribute_list = new ArrayList();

    java.util.Enumeration enum2 = request.getParameterNames();
        while (enum2.hasMoreElements()) {
            paramName = (String) enum2.nextElement();
            boolean isNewQ = paramName.startsWith("qID_");

            if (isNewQ) {
                attribute_id = paramName.replaceAll("qID_", "");
                YesNoNAAnswer clr = new YesNoNAAnswer();

                if (request.getParameter("radio_" + attribute_id) != null) {
                    radioValue = request.getParameter("radio_" + attribute_id);
                } else {
                    radioValue = "0";
                }

                if (request.getParameter("textbox_" + attribute_id) != null) {
                    textValue = request.getParameter("textbox_" + attribute_id);
                } else {
                    textValue = "";
                }

                if (request.getParameter("check_" + attribute_id) != null) {
                    radioValue = request.getParameter("check_" + attribute_id);
                } else {
                    // checkValue = "";
                }

                if ("0".equals(radioValue) || "2".equals(radioValue)) {
                    strStatus = "0";
                }

                strStoredNo = request.getParameter("stored_" + attribute_id);
                if ("2".equals(radioValue) && !"yes".equals(strStoredNo)) {
                    deficiencyFound = true;
                }

                clr.setWorkflow_instance_id(ci.getWorkflow_instance_id());
                clr.setWfi_work_item_action_id(ci.getWfi_work_item_action_id());
                clr.setFail_reason(textValue);
                clr.setAttribute_id(attribute_id);
                clr.setToggle_value(radioValue);
                answer_attribute_list.add(clr);
            }
        }

        ci.setChecklist_state(strStatus);
        ci.setLast_update(dateNow);
        ci.setAdditional_info(FormUtil.getFieldValue(request, FIELD_ADDITIONAL_INFO));

        processUpdateCheckbox(request, ci, currentActionItem);

            // Update the base check list
            ciDao.updateInstance(ci, authenticatedUser);

            // Update the check list question responses
            ynnDao.updateToggles(answer_attribute_list, authenticatedUser);

            // update the work flow
            WorkflowInstanceDAO wfiDao = new WorkflowInstanceDAO();
            WorkflowInstanceForm wfiForm = new WorkflowInstanceForm(wfiDao, authenticatedUser);
            WorkflowInstance wfi = (WorkflowInstance) wfiForm.view(ci.getWorkflow_instance_id(), authenticatedUser);
            wfiForm.updateWorkFlowInstance(wfi, currentActionItem);

            setFormFeedback("You have successfully updated the checklist.");
            triggerUpdateEmail(request, ci, wfi, currentActionItem);

    return ci;
}

score 1 · Accepted Answer

まず最初に、手続き型プログラミングではなく、オブジェクト指向プログラミングを行っているため、どのクラスがどの責任を負うべきかを事前に考えておく必要があります。

それで、ここでの責任は何ですか？独立して実行する必要がある次のタスクをリストできます。

ユーザー入力データの検証: HTTP を介してクライアントから提供された値の有効性 (値の範囲、無効な値、セキュリティ上の問題 (インジェクション保護)...) を確認します。
ドメインモデルメソッドへの呼び出しのリストのみを含むコントローラー。
ドメインモデル: ビジネスデータを表し、操作メソッドを提供する一連のクラス。
ドメインモデルを永続化する手段: データベース、XML ファイルへ...

すべての処理の開始から終了まで、(必要に応じて)トランザクションを管理することを忘れないでください。

無駄な依存関係を削除する: ドメインモデルは、HTTP とデータの送信元を認識しないようにする必要があります。コントローラーは、データがどのように永続化されるかを認識してはなりません。

車輪を再発明しないでください。たとえば、検証とコントローラーのニーズには、Struts 2などの MVC フレームワークを使用してください。hibernate/JPAのような永続化のためのフレームワークを使用します。

score 0 · Accepted Answer

クイックフィックス:

この関数で何が起こっているかを手で簡単な図にします
実行されるアクションの種類ごとに 1 つの小さな関数を記述します。
からのすべての貴重なパラメーター値を含むクラスを作成します。HttpServletRequest
そのようなクラスのオブジェクトをHttpServletRequest(コンストラクター、静的メソッド、セッターなど、必要に応じて)から作成するメソッドを作成します。
UI コンポーネントに状態を使用できます。

擬似コードは次のようになります

  HttpServletRequest req;
  RequestParameters params = new RequestParameters(req);
  //make some processing here (database)
  for(UIState state : stateList){
    state.makechanges(params)
  }
  /* UIState can be an interface, each UI item has it own subclass*/

java - この Java メソッドをリファクタリングして OOP をより有効に活用する方法

2 に答える 2

Related

Reference