第一貨物 [Mirror]

※まず、他のコードレビューの内容と同類のものは対応してください。その後レビューします。

※まだレビュー中です。

・設計書　全てのシートで変な拡大率で保存するのはやめてください。
　→例えば、お客様にExcelファイルを送るとして、途中のシートの途中の位置から表示されたらどのように思うでしょうか？

・設計書　画面項目仕様　「mode2 取得先」だけあり、「disp2」がないのはおかしいです。

・設計書　画面レイアウト、画面項目仕様　エクセルの13行目などに謎の行が存在しています。

　推測するにサブタイトルのようなラベルなのかと思います。
　画面に表示するのであれば、「label」で設定すれば良いのではないでしょうか？
　そもそも、ルールが決まっていないのであれば、どのように書くべきか相談が必要かと思います。ルールを決めるのか、決めないのか
　【】ではなく＜＞ですし
　それともメモ行ですか？

　
・設計書　まず、全体的に、書き方を統一してください。
　　　　　　例：文字列を""で囲うのか、囲わないのか（囲ってください）
　　　　　　　　AND　And　and & && 且つ　→　紛らわしい表現を避ける
（and 且つ　が一般的かと。&はビット演算のイメージが強い &&は言語に依存している)
（or または が一般的かと。|は罫線か何かわからない。||は言語に依存している)

　データチェック仕様　No34など　締日１支払日１　
　「(締日１ = 0 かつ 支払日１ = 0) または
　　(締日１ != 0 かつ 支払日１ != 0)
　　以外の場合」
　と記載していますが、もっと明確に間違いないように記載しましょう。「以外の場合」が全体に掛かるのか、直前に掛かるのか。
　今回では、項目が二つで、意味をしっかり考えればわかりますが、項目が増えたり、判定文に矛盾が無ければ、誤った仕様で実装してしまいます。
　基本的に、設計書は他人が見るものです。誤解なく伝わるように記載してください。
　この場合、
　「((締日１ = 0 かつ 支払日１ = 0) または、(締日１ != 0 かつ 支払日１ != 0))以外の場合」
　改行で切り分けたなどは、セルの幅で改行を入れただけとか、いろいろ解釈できます。最終的な時間ロスに繋がります。

・データチェック仕様　No.20 No.21

「2つ以上の項目に同じ値が入っている
　("0000000"以外)」
こちらはOKかと思いますが、直すなら「"0000000"以外の値かつ、同じ値の項目が存在するの場合」とかかと思います。
「┌一部だけが0000000(全部0000000は正常)の場合
　｜　┌全部がオール8
　｜　｜　集金 ERR3 '2729P'
　｜　└それ以外
　└　　　集金 ERR4 '2729N'」
こちらはNG
案１、値が"0000000"の項目が1つまたは、2つの場合
案２、全ての項目が"0000000"ではなく、いづれかの項目が"0000000"の場合
など
全部→全ての項目
オール8→全ての桁が"8"の場合
全ての項目で全ての桁が"8"の場合
全部とか、オール8とか、伝わらなくはないですが、QAの発生する原因ですよね。
＞全体的に抽象的な表現過ぎます。
集金 ERR3 '2729P'　→ '2729P'『集金 ERR3 』
「集金 ERR3」はコメントですよね？
実際に設定してほしいのは'2729P'ですよね？　コメントの記載方法はルールで決まっています。

・プロパティ一覧　変数　郵便番号フラグとは？　未使用ですが必要ですか？

・プロパティ一覧　変数　照会遷移区分とは？　未使用ですが必要ですか？

・プロパティ一覧　変数　顧客コードとは？　未使用ですが必要ですか？

・プロパティ一覧　変数　初期表示データとは？　未使用ですが必要ですか？

・プロパティ一覧　データ　請求書出力店所情報　他のエンティティも同様

　メソッドが異なる旅に取得するのはやめてください。　※場合にもよりますが
　例：mode2
　　　　getDataMode2（データ取得処理）　・・・これはデータを取得するために準備したブロックですので、理由が無ければ基本的に、ここで取得しましょう。
　　　　・戻り値や引数
　　　　　　recordクラス、AtomicReferenceクラス（参照渡し）などを使用する。→わからなければ聞く
　　　　　validateMode1（チェック処理）　・・・引数等で取得したデータを渡してください。
　　　　　dataProcessMode1（データ処理） ・・・引数等で取得したデータを渡してください。
　　　　　okProcessMode1（ＯＫ処理）　　　・・・引数等で取得したデータを渡してください。

・PG 889行目～　　　「"" + 」は、なんのためなのでしょうか？ Integer型には、toString()がありますよね？

		//金額帯１上限
		form.setND0800040("" + syokaiDt.getKSHFMAXK1());

・F2　設計書と実装が合っていません。

・設計書の何処と一致するのでしょうか？　また、コメントと合っていません。何が正解なのでしょうか？

・データチェック仕様　実装と合っていない
　実装では1から4までの4回繰り返しとなっている。
　javaの配列は「0」から始まっている。
　１つ目の要素(インデックス0)は見ていない　正しく動作していますか？

・設計書と実装処理が異なる
　データチェック仕様の下のところ？　
　そもそも内容もあっていない　''OK01 '？　'OK '

・f3を押すとND10020でエラーになる

※ND10030とND10080間違えました。後で消します。　渡邉

2025/05/08
・コメント「顧客台帳入力選択」→「顧客基本情報修正選択」

/**
 * 顧客台帳入力選択
 */
@Controller
@RequestMapping("/ND10030")
public class ND10030Controller extends BaseController {

・２つ目以降もjavadocを記載する。

・参照渡しはAtomicReferenceを使用してください。
https://qiita.com/TsuyoshiUshio@github/items/9cd8ebf76a38198cd332

  // 参照渡しするためにクラスを作成
  class MutableInteger {
    public Integer value;

    public MutableInteger(Integer value) {
      this.value = value;
    }
  }

・197行目　不要ならば消してください。

    // messageList.put(" ", "修正入力は取り消されました。今、入力したデータは反映されません。");

・getterは不要ではないでしょうか？どんな意味がありますか？
Java の record においては、明示的に getter を定義する意味は基本的にありません。なぜなら、record はフィールドを宣言した時点で自動的にアクセサメソッド（getter相当）を生成するからです。

✅ 結論（要点）
record には自動で public なアクセサメソッドが生成される
フィールド名がそのままメソッド名になる（例：id()、name()）
通常のクラスでいう getXxx() ではないが、実質 getter と同等
よって、自分で getter を書く必要はほとんどない

 private record VariableDataList(
      Boolean CommDtAddFlg,
      String kousinDtTekiyouYMD,
      String kousinDtUpdateHm,
      String targetDtKbn,
      String kokyakuAtukaiCd1,
      String kokyakuAtukaiCd2,
      String kokyakuAtukaiCd3,
      String kokyakuAtukaiCd4,
      String jigyouKbn,
      String deleteKbn,
      Boolean resetFlg) {

    private Boolean getCommDtAddFlg() {
      return CommDtAddFlg;
    }

    private String getkousinDtTekiyouYMD() {
      return kousinDtTekiyouYMD;
    }

    private String getkousinDtUpdateHm() {
      return kousinDtUpdateHm;
    }

    private String gettargetDtKbn() {
      return targetDtKbn;
    }

    private String getkokyakuAtukaiCd1() {
      return kokyakuAtukaiCd1;
    }

    private String getkokyakuAtukaiCd2() {
      return kokyakuAtukaiCd2;
    }

    private String getkokyakuAtukaiCd3() {
      return kokyakuAtukaiCd3;
    }

    private String getkokyakuAtukaiCd4() {
      return kokyakuAtukaiCd4;
    }

    private String getjigyouKbn() {
      return jigyouKbn;
    }

    private String getdeleteKbn() {
      return deleteKbn;
    }

    private Boolean getresetFlg() {
      return resetFlg;
    }

・344行目　フィールド変数です。NG

VariableDataList vdl = new VariableDataList(false, "", "", "", "", "", "", "", "", "", true);

・346行目　中身が1つなら「record」クラス不要では？

  /**
   * COMM_AREA明細.COMM_STVKCUSTW[顧客基本情報_変更情報] レコードクラス
   * 
   * @param stvkcustw COMM_STVKCUSTW[顧客基本情報_変更情報]
   */
  private record CurrentSTVKCUSTW(String stvkcustw) {
    private String getdata() {
      return stvkcustw;
    }

    private CurrentSTVKCUSTW setvkcustw(String value) {
      return new CurrentSTVKCUSTW(value);
    }
  }

・361行目　フィールド変数です。NG

CurrentSTVKCUSTW comm_vkcustw = new CurrentSTVKCUSTW("");

・378行目～719行目　コメントアウトされてますが、なんなのでしょうか？

  // /**
  // * f3, f4 ファンクションボタン処理 （設計書ママ）
  // *
  // * @param id
  // * @return
  // * @throws Exception
  // */
  // protected String ND510F3(Model model, BigDecimal id) throws Exception {

・メソッド名を決めてからコードレビューお願いします。

  // TODO メソッド名考えないと
  private String methodX(Model model, BigDecimal id) {

・776行目 処理合ってますか？
idで消すのは1レコードだけですが？
このidは1ユーザーの画面IDで、コミュエリアのIDではないですよね？
「ND10010」はグループキーですよね？
基本的に「commAreaDtService」は直接使用しないでください。→CommAreaUtilityが存在します。

      // OSCOMADT[COMM_AREA明細].ND10010のデータを削除する。
      commAreaDtService.deleteById(id);

・793行目　謎のコメントです。適切な内容に変えてください。

          // 次のループへ
          // continue;

・749行目　

vdl = vdl.setresetFlg(false);

書き方がいまいち。
下記、セッターをわざわざ作成するのであれば、そのタイミングでインスタンスの再設定まで行ってしまえば、上記で再設定する必要がないのでは？

    private VariableDataList setresetFlg(Boolean value) {
      return new VariableDataList(this.CommDtAddFlg, this.kousinDtTekiyouYMD, this.kousinDtUpdateHm, this.targetDtKbn,
          this.kokyakuAtukaiCd1, this.kokyakuAtukaiCd2, this.kokyakuAtukaiCd3, this.kokyakuAtukaiCd4, this.jigyouKbn,
          this.deleteKbn, value);
    }

・844行目　メソッド名を決めてからソースレビューをお願い致します。

  // TODO メソッド名考えないと
  private void methodY(Model model, BigDecimal id) {

・2014行目　本当に「popBackId」で合っていますか？前回履歴からPOPしたら消えますが。
見るだけであれば、「getRecentBackId」ではないでしょうか？　（※仕様はわかりませんので正しいければ良いです）

String recentId = CommAreaUtility.popBackId(getCommArea().get(), "/ND10020/u/" + id);

・pvmode23　名前NG　pvMode23

・引数はrecord1つ渡せば、ほぼ足りるのではないでしょうか？

  private BindingResult checkIsEmptyMode2(MPND020Form form, BindingResult bindingResult,
      Optional<OVJYUGY> kakuJyuDt, Optional<OVJYUGY> tantJyuDt, Optional<RDTENS> nyuuryokuTenDt,
      Optional<RDTENS> kokyakuTenDt1, Optional<RDTENS> kokyakuTenDt2, Optional<RDTENS> kokyakuTenDt3,
      Optional<RDTENS> kokyakuTenDt4, Optional<RDCODE> gyousyuName, Optional<VKCHINM> syozaitiNameDt,
      Optional<VKCHINM> seikyuutiNameDt) {