第一貨物 [Mirror]

リフレクションを多用しない！！！
「invoke」

→　※配列の対応も合わせて対応する。

どうしてもの時で、今回は違う。
書けばいいだけ。

Java のリフレクションは非常に強力ですが、使い方を誤ると 保守性・安全性・パフォーマンス に悪影響を与えます。主な注意点をまとめます。

🔎 リフレクション利用時の注意点¶

1. カプセル化の破壊 ¶

• private フィールドやメソッドにアクセスできてしまう (setAccessible(true))。
• 設計者の意図を無視してしまい、クラスの変更に弱くなる。

👉 本当に必要な場合以外は避ける。特にライブラリや外部クラスでは注意。

2. パフォーマンス低下 ¶

• 通常のメソッド呼び出しよりもオーバーヘッドが大きい。
• 頻繁に呼び出す処理（ループの中など）では顕著に遅くなる。

👉 キャッシュ（Method や Field オブジェクトの再利用）を行う、または MethodHandle や VarHandle を検討する。

3. 型安全性の欠如 ¶

• コンパイル時チェックが効かず、実行時に ClassCastException や NoSuchMethodException が発生しやすい。
• ジェネリクスの情報も一部は消失しているため、誤用を検知できない。

👉 instanceof や null チェックを丁寧に行う。

4. セキュリティ制約 ¶

• セキュリティマネージャー（Java 17 で deprecated）やモジュールシステム（Java 9 以降）の制約により、アクセスが制限される場合がある。
• JDK 内部 API へリフレクションアクセスすると、Java 17+ では警告やエラーになるケースがある。

👉 公開 API で代替できないかを検討する。

5. モジュールシステムとの相性 ¶

• Java 9 以降、--illegal-access オプションなしでは内部パッケージにアクセスできない。
• 例えば java.base の非公開クラスにアクセスすると InaccessibleObjectException。

👉 opens 宣言や --add-opens オプションで対応が必要。

6. メンテナンス性の低下 ¶

• ソースコードを見ただけでは利用されているメソッドやフィールドが分からない。
• 将来のリファクタリング（メソッド名変更・シグネチャ変更）で簡単に壊れる。

👉 リフレクションは「最後の手段」として使う。

7. 代替手段の検討 ¶

• ServiceLoader や インターフェース + 実装クラス で代替できる場合が多い。
• フレームワークを使うなら、DI（Spring / CDI）を利用する方が安全。

✅ まとめ¶

リフレクションは便利ですが、

• 性能劣化
• 安全性低下
• メンテナンス性低下

といったリスクを伴います。
👉 「どうしても必要なケース（フレームワーク・ライブラリ内部処理など）」以外は極力避け、代替手段を検討するのがおすすめです。

ご希望なら、「安全にリフレクションを使うためのベストプラクティス（例：キャッシュ方法、例外処理のテンプレート）」 も整理できますが、見ますか？

設計書と実装を合わせる

間違っているのであれば、設計者に　連絡するべき

設計書に存在しない（且つ、使用しうる）IDを勝手に追加実装しない（設計・実装が合っていません。）
「ND1400065」以降の値全て

パターン１・・・実装したIDを全て設計書に記載する

パターン２・・・配列での実装にする

・項目が追加されたら、ND1400065を使用しますよね？
　後から知って、最後のIDを調べる？PGを見る手間が必要がでますよね？NGです。

・明細に項目追加されたら、修正するのは、非常に大変ですよね？

・PG　各IDにコメントが無く、項目が不明です。→見てわからなくなりますよね？

N件（配列）での実装にしてください。
http://10.104.253.197/projects/3-dos/wiki/%E7%94%BB%E9%9D%A2%E3%80%80N%E4%BB%B6%E3%83%87%E3%83%BC%E3%82%BF%E3%82%92%E8%A1%A8%E7%A4%BA%E3%83%BB%E5%85%A5%E5%8A%9B%E3%81%99%E3%82%8B%E7%94%BB%E9%9D%A2%E3%81%AB%E3%81%A4%E3%81%84%E3%81%A6

そもそも、設計→実装で
どう書くべきか、不明（確信をもって実装できない）なら聞くべきです。

恐らく、設計者の意図と異なります。（設計書の作成ルールに沿っていませんが・・・）

なぜ、このような実装になったのか？

作成を始める時、ちゃんとツールから、コード生成していますか？

しているのであれば、ツール生成日時等は消さない

データ処理

長いのであればメソッド「dataProcessModeX」に記載して欲しいです。

セルフ確認不足　不要コメント消す

      // throw new Exception("契約方面データ取得メソッド エラー");

コーディング　コメントについて

「　空白（非表示）」が消されてしまうと設計書と合っているのか、設計書を見ないわからない。
この案件では、「全てコメントに残す」とルールになっています。
他、いろいろな箇所

コメントは、書く時に使用するのではなく、第三者が読む時に使用します。

無駄に長いので簡潔に記述してください。

Arrays.asList("XXX", "XXX").contains(

ここまでOK¶

指摘いただいた部分のPG、設計書の修正が完了しました。
再レビューよろしくお願いいたします。

設計書と実装が異なる

設計書通りであれば、1～？？までループする

実装では、0～？？までループする

そもそも「プロパティ.契約方面データを読み込み、以下をループする。」で何回ループするか明記されていない。
→無理な拡大解釈　なぜhomenKosuまでのループと判断できたか？？？

PL/1 配列のインデックスは「1」から開始
Java 配列のインデックスは「0」から開始（現代的な言語はすべて。リビルド後の設計書はこれを基本とする）
→参照するインデックスを実装レベルで勝手に変えない

※N件目やXXXX[i]などについて、辻褄を合わせるのはよいが、
　カウントアップの処理を改変するのはNG

設計書通りなら
0から始めてカウントアップする→照会する場合カウンタ-1でアクセスすべきでは？？

その後の処理でカウンタを違う使い方をするかもしれない
・全体を読まないと合っていると判断できなくなる。可読性が悪い（NG）
・処理の辻褄が合わなくなる可能性が出る（NG）
・修正時の設計書通りに直せなくなる（NG）

実装を直すべきか？
設計書を直すべきか？

""と判断しない　※何度か同じような指摘をしているかと思います。

!"".equals(stvkcun1w.getKHOMSRKM()

桁数１で、普通、文字数0の空は入らない想定かと思います。

!stvkcun1w.getKHOMSRKM().isBlank()

↓

    nd10070Data.setGamenTekiyouKosu(toInt(form.getND1400005()));

下記の処理は設計書のどこの内容か？

これでダメなのか？

    for (int i = 0; i < 20; i++) {
      kousinArrayKosu[0] = toInt(getFormValue("ND1400012", form, 1));
    }

記述が無駄に長い
画面レイアウトの処理の流れを追いにくい
→コーダーとして、工夫すべきポイント。
動けばいい・・・初心者

長々と記述する必要があるなら、メソッドに切り分けるべき。

コーディング初心者とベテランの違いは、単に「コードが書けるか書けないか」ではなく、考え方・習慣・質の差にあります。順に整理してみます。

1️⃣ コードの書き方¶

2️⃣ 設計・構造の理解¶

3️⃣ デバッグ・保守性¶

4️⃣ 思考の違い¶

• 初心者

  • 「とりあえず動くコード」を書く
  • 実装重視で全体像はあまり考えない

• ベテラン

  • 「動かすだけでなく維持できるコード」を書く
  • 仕様・性能・拡張性・テストを意識

💡 まとめ

• コーディングスキルは「コードを書く速さ」だけでなく、「読みやすさ・保守性・設計の深さ」に現れる
• ベテランほど「未来の自分や他人が読むこと」を意識して書く

もし希望なら、初心者がベテランっぽく見える 具体的コーディングの改善例 を短いサンプルで示すこともできます。
やってほしいですか？

コメントを消さない

コメントを消さない

無駄に長い

個数上限区分１～２０で同じ処理するパターンが多くあるのですよね？

個数上限区分１～２０のIDをリストで取得するメソッドを作成してください。

コメントを消さない
「『DB更新仕様　運賃基本情報更新時（上限区分）　参照』」

メソッド名
「updateSTVKCUN1WJyougenKbn」
CommAreaを更新するニュアンスが入ってない
http://10.104.253.197/projects/3-dos/wiki/%E3%83%A1%E3%82%BD%E3%83%83%E3%83%89%E5%90%8D

テーブル名：「COMM_AREA明細.ND10050.COMM_VKCUN1W」とは？
存在しない
勝手に空気を読んで実装しない。→設計者にQA（設計書修正）すべき内容

そもそも「マッピング仕様」に書くべき

COMM_AREA明細.ND10050.COMM_VKCUN1W[顧客マスタ(運賃基本情報)_変更情報]を更新する。
『DB更新仕様　運賃基本情報更新時（上限区分）　参照』

マスタメンテ更新データを設定する　『マッピング仕様　運賃基本情報更新時（上限区分）　参照』
COMM_AREA明細.ND10050.COMM_VKCUN1W[顧客マスタ(運賃基本情報)_変更情報]　←　マスタメンテ更新データ

下記も同じ

COMM_AREA明細.ND10050.COMM_VKCUN2W[顧客マスタ(運賃方面別情報)_変更情報]のデータ行数分、																																						
COMM_AREA明細.ND10050.COMM_VKCUN2W[顧客マスタ(運賃方面別情報)_変更情報]を更新する。																																						
																					『DB更新仕様　運賃方面別情報更新時（上限区分）　参照』																	

謎のコメント　要る？要らない？
// throw new Exception("");

「処理を終了する。」とは？
ルールでは、どのように実装するかわからないと思うが
なぜこの実装になったのか？　

何度も言っているが、勝手に（仕様の妄想して）実装しない
「設計書→誰でも同様に動くように記載する」がルール

下記どこから出て来たのか？？？？

    // 画面の項目表示を更新
      checkQuitReview(form, bindingResult, model, httpSession, stvkcun1w);

読みにくい

「dataProcessMode4」メソッドをなぜ作らないのか？

方面コード　いろいろNG

「2次元配列」とは記載していない→実装と設計が合っていない

「更新FLG」とは？　

勝手に2次元配列にしない
インデックスが具体的に指定されている。勝手に変えない

ちなみに「プロパティ.方面コード（変数.ループカウンタ）.更新FLG ← "DEL"」と言う表現を尊重すると

new String[2][45];・・・×
new String[45][2];・・・せめてこっち

0・・・方面コード
1・・・更新FLG
勝手に決めない　コメントが無い　自己満足で終わっていませんか？

構造体、Mapなどの配列では？　妄想しない！！！

何度も言うが勝手に実装しない
　動くように作る・・・今回のゴールではない
　設計書通り作る（設計書の辻褄も合っている）がゴール

定数・・・コーディングルール・一般的な書き方に習ってください。

「final」

すべて大文字

単語の区切りはアンダースコア _

意味がわかる名前を付ける

UNTIN_SANTEI_TEKIYOU_CODE

javaの変数の記述方法は？　合ってないですよね

※拡大解釈しないでください。

・選択は複数あります。勝手にループしないでください。

・実装ではループしていますが、最終行のU、R、Xの入力により遷移先が変わる仕様なのでしょうか？

→PL/1（第一貨物）では、入力された先頭の行を見ているはずです。

QAが出ずに実装完了はNGです。

レビュー頂いた内容で修正いたしましたので、確認お願いいたします。

またお伝え損ねておりましたが、mode5の開発が完了したので改めてレビューお願いいたします。

多分

    if (toInt("46").compareTo(keiyakuHomenData.size()) <= 0) {

< → <=

結局、最終行が採用されるで合っているのでしょうか？

こちらは、先頭のようですが・・・

「画面.選択に"U" OR "R" OR "X" が入力されている行」が複数あった場合、どうなるのでしょうか？
設計書からは読み取れませんが。
「画面.選択の入力値で処理を分岐。」
からも、先頭行を使用する内容はありません。

ちなみにカンマ編集は

String[] arr = {"apple", "banana", "cherry"};
String result = String.join(",", arr);

レビューありがとうございます。

#37を修正しました。

#38についてですが、画面.選択に"U" OR "R" OR "X" が複数行入力された場合は validatemode5 のチェック処理でエラーとしています。
よって画面.選択の入力値が存在する行は1行だけですので、先頭を採用する処理にしています。