clean-abap

Clean ABAP

このスキルでは、ABAP 向けに Robert C. Martin の Clean Code を適用した Clean ABAP スタイルガイドに基づき、Clean ABAP の原則に照らして ABAP コードを包括的にチェックします。

このスキルの使い方

ABAP コードが Clean ABAP に準拠しているかチェックするときは、次の手順に従います。

1. ユーザーが提供したコードを読みます
2. Clean ABAP の各セクション（名前、言語、定数、変数、テーブル、文字列、ブール値、条件、IF、クラス、メソッド、エラー処理、コメント、書式、テスト）ごとに問題を分類します
3. 可能な場合は具体的な行番号を示して、違反を特定します
4. 問題のあるコードと、改善後のコード例を示して、実行可能な推奨事項を提示します
5. 影響度（重大、高、中）に応じて問題に優先順位を付けます

チェック項目

1. 名前

主な原則：

• 内容や意味が伝わる、説明的な名前を使います
• 解決策の領域や問題領域の用語を優先します
• 発音しやすい名前を使います
• 一貫してスネークケースを使います
• 必要でない限り、略語は避けます
• クラスには名詞、メソッドには動詞を使います
• 「data」「info」「object」のような意味の薄い語は避けます
• 概念ごとに使う単語を一つに決めます
• エンコーディング（ハンガリアン記法、iv*、rv*、lt_ などの接頭辞）は避けます
• 組み込み関数と紛らわしい名前は避けます

チェックする項目：

• 説明的でない変数名、メソッド名、クラス名（例：data1、temp、x）
• コード内で一貫していない略語
• 命名規則の混在（スネークケース以外の形式）
• 名前に含まれる意味の薄い語
• ハンガリアン記法や不要な接頭辞（iv*、ev*、rv*、lt*、ls_）
• ABAP の組み込み関数と紛らわしいメソッド名

2. 言語

主な原則：

• 手続き型プログラミングよりオブジェクト指向を優先します
• 手続き型の言語構造より関数型の言語構造を優先します
• 古い言語要素は避けます
• デザインパターンは適切に使います

チェックする項目：

• 古い構文の使用（SELECT でのホスト変数のエスケープ漏れなど）
• オブジェクト指向にすべき手続き型コード
• 代入の代わりに古い形式の MOVE を使用
• to_upper()/to_lower() の代わりに TRANSLATE を使用
• NEW の代わりに CREATE OBJECT を使用
• テーブル式の代わりに古い形式の READ TABLE を使用

3. 定数

主な原則：

• マジックナンバーの代わりに定数を使います
• 定数には説明的な名前を付けます
• 定数インターフェースより ENUM を優先します
• 関連する定数をグループ化します

チェックする項目：

• コード内のマジックナンバーや文字列リテラル
• 説明的でない名前の定数（c_01、c_x など）
• BEGIN OF/END OF ブロックにまとめるべき、グループ化されていない定数

4. 変数

主な原則：

• 先行宣言よりインライン宣言を優先します
• 宣言ブロックの外で変数を使わないようにします
• 先行宣言を連結しないようにします
• 動的なデータアクセスにフィールドシンボルを使わないようにします（モダン ABAP）
• 適切なループ対象を選びます（フィールドシンボル、参照、値）

チェックする項目：

• インライン宣言のほうが明確な場合に、DATA を先行宣言している
• 宣言ブロックのスコープ外で変数を使用している
• DATA 宣言を連結している
• ASSIGN に不要なフィールドシンボルを使っている

5. テーブル

主な原則：

• 適切なテーブル型（STANDARD、SORTED、HASHED）を使います
• DEFAULT KEY は避けます
• APPEND TO より INSERT INTO TABLE を優先します
• READ TABLE や LOOP AT より LINE_EXISTS を優先します
• LOOP AT より READ TABLE を優先します
• ネストした IF より LOOP AT WHERE を優先します
• 不要なテーブル読み取りは避けます

チェックする項目：

• DEFAULT KEY を使うテーブル
• INSERT INTO TABLE のほうが適切な場合に APPEND TO を使用
• LINE_EXISTS のほうが明確な場合に READ TABLE ... TRANSPORTING NO FIELDS を使用
• READ TABLE が意図されている箇所で LOOP AT ... EXIT を使用
• WHERE 句で対応できる箇所で、LOOP AT 内にネストした IF を使用
• 存在確認後に再度読み取る、二重の読み取り

6. 文字列

主な原則：

• 文字列リテラルの定義には `（バッククォート）を使います
• テキストの組み立てには |（パイプ）を使います

チェックする項目：

• 文字列リテラルにシングルクォートを使っている
• 文字列テンプレートの代わりに && で文字列を連結している

7. ブール値

主な原則：

• ブール型には ABAP_BOOL を使用します
• 比較には ABAP_TRUE と ABAP_FALSE を使用します
• ブール変数の設定には XSDBOOL を使用します
• ブール値が適切な選択肢かどうか（列挙型と比較して）を検討します

確認項目：

• ABAP_BOOL の代わりに CHAR1 やその他の型を使用していないか
• ABAP_TRUE／ABAP_FALSE の代わりに 'X' や ' ' と比較していないか
• ブール値の設定に XSDBOOL の代わりに IF-THEN-ELSE を使用していないか
• 分割すべきブール型パラメーターがないか

8. 条件

主な原則：

• 条件は肯定形にするよう努めます
• NOT IS より IS NOT を優先します
• ブール値を返すメソッド呼び出しには、述語形式を検討します
• 複雑な条件の分解や抽出を検討します

確認項目：

• 肯定形にできる否定条件がないか
• IS NOT の代わりに NOT IS を使っていないか
• 分解すべき複雑な入れ子条件がないか
• メソッドに抽出すべき長い条件がないか

9. If 文

主な原則：

• 空の IF 分岐を作りません
• 複数の選択肢がある場合は ELSE IF より CASE を優先します
• ネストの深さを抑えます

確認項目：

• ロジックが ELSE にしかない空の IF がないか
• CASE にすべき複数の ELSE IF がないか
• 深くネストした IF 文（3 レベル超）がないか

10. クラス

主な原則：

• 静的クラスよりオブジェクトを優先します
• 継承よりコンポジションを優先します
• 同じクラスで状態を持つ処理と持たない処理を混在させません
• デフォルトではグローバルにし、適切な場合にのみローカルにします
• 継承を想定していない場合は FINAL にします
• メンバーはデフォルトで PRIVATE とし、必要な場合にのみ PROTECTED にします
• ゲッターの代わりに不変オブジェクトを検討します

確認項目：

• インスタンスベースにすべき静的クラスがないか
• 深い継承階層になっていないか
• 状態を持つメソッドと持たないメソッドが混在していないか
• 継承を想定していないのに FINAL ではないクラスがないか
• PRIVATE／PROTECTED にすべき PUBLIC メンバーがないか
• 不変データに対して不要なゲッターメソッドがないか

11. メソッド

主な原則：

• 静的メソッドよりインスタンスメソッドを優先します
• パブリックなインスタンスメソッドではインターフェースを実装します
• IMPORTING パラメーターは少数（3 個未満）に抑えます
• OPTIONAL パラメーターを使う代わりにメソッドを分割します
• RETURN、EXPORT、または CHANGE で扱うパラメーターは正確に 1 つにします
• EXPORTING より RETURNING を優先します
• 1 つのことだけを、うまく、そしてそれだけを行います
• メソッドは短く保ちます（理想は 3～5 ステートメントです）
• 速やかに失敗させます
• 可能であれば RECEIVING、EXPORTING キーワードを省略します
• インスタンスメンバーの呼び出しでは自己参照 ME を省略します

確認項目：

• インスタンスメソッドにすべき静的メソッドがないか
• インターフェースの一部ではないパブリックメソッドがないか
• IMPORTING パラメーターが 3 個を超えるメソッドがないか
• 複数の OPTIONAL パラメーターがないか（メソッドを分割すべきです）
• 出力パラメーターが複数ないか
• RETURNING ではなく EXPORTING を使用していないか
• 長いメソッド（20 行超）がないか
• 複数の処理を行うメソッドがないか
• 不要な RECEIVING、EXPORTING キーワードがないか
• 明示的な ME-> 呼び出しがないか

12. エラー処理

主な原則：

• 戻りコードより例外を優先します
• クラスベースの例外を使用します
• 対処可能な例外には CX_STATIC_CHECK をスローします
• 回復不能な状況では CX_NO_CHECK をスローします
• RAISE EXCEPTION TYPE より RAISE EXCEPTION NEW を優先します
• 外部の例外をラップします

確認項目：

• 例外ではなく戻りコードを使用していないか
• エラー処理にメッセージクラスを使用していないか
• 古い形式の RAISE EXCEPTION TYPE を使用していないか
• 外部の例外をラップせずに扱っていないか
• 汎用的な CX_ROOT をキャッチしていないか

13. コメント

主な原則：

• コメントではなくコードで表現します
• コメントは不適切な名前を正当化するものではありません
• 何をするかではなく、なぜそうするのかを説明するコメントを書きます
• コメントには * ではなく " を使用します
• コードをコメントアウトする代わりに削除します
• FIXME、TODO、XXX には自分の ID を付けます
• ABAP Doc はパブリック API にのみ使用します

確認項目：

• コードの動作を説明するだけの明白なコメントがないか
• 不適切な名前を補うコメントがないか
• コメントアウトされたコードがないか
• " の代わりに - を使ったコメントがないか
• TODO／FIXME タグのないコメントがないか
• コメント内で手動でバージョン管理していないか
• コメント内にメッセージテキストが重複していないか

14. 書式設定

主な原則：

• 有効化する前に ABAP Formatter を使用します
• 1 行につき文は 1 つまでにします
• 適切な行の長さにします（120 文字）
• 区切りには空行を 1 行だけ使用します（それ以上は不可）
• 閉じ括弧は行末に置きます
• パラメーターが 1 つの呼び出しは 1 行にまとめます
• タブを使ってインデントし、位置を揃えます

次の点を確認します：

• 1 行に複数の文がないか
• 120 文字を超える行がないか
• 空行が複数連続していないか
• インデントが一貫しているか
• 連鎖代入がないか

15. テスト

主な原則：

• テスト可能なコードを書きます
• プライベートな内部実装ではなく、公開部分をテストします
• given-when-then の構成を使用します
• アサーションは少数に絞り、焦点を明確にします
• 適切な種類の assert を使用します

次の点を確認します：

• テストできないコード（密結合、依存性注入がない）
• given-when-then の構成が明確でないテスト
• 関連性のないアサーションが複数ある
• 公開メソッドにテストクラスがない

出力形式

分析は次の形式で構成してください。

# Clean ABAP Check Results

## Summary
- Total Issues: [count]
- Critical: [count]
- Major: [count]
- Minor: [count]

## Critical Issues

### [Category] - [Issue Title]
**Location:** Line [X] / Method [name]
**Problem:** [Description of what violates Clean ABAP]
**Recommendation:** [How to fix it]

**Anti-pattern:**
```abap
[problematic code]

クリーンなコード：

[improved code]

主な問題

[重大な問題と同じ形式]

軽微な問題

[重大な問題と同じ形式]

良い点

• [Clean ABAP の観点で適切にできている点]

総合評価

[コード品質と主な改善点の簡潔な概要]

## Priority Levels

**Critical:** Issues that significantly impact maintainability, testability, or could cause bugs
- Magic numbers without constants
- Deep nesting (>3 levels)
- Methods with >5 parameters
- Empty IF branches
- Static classes without interfaces
- Return codes instead of exceptions

**Major:** Issues that reduce code clarity and violate Clean ABAP principles
- Non-descriptive names
- Hungarian notation/prefixes
- Long methods (>20 lines)
- DEFAULT KEY usage
- Commented-out code
- Multiple output parameters

**Minor:** Issues that are stylistic improvements
- Missing inline declarations
- String quotes instead of backticks
- Unnecessary ME-> references
- Formatting inconsistencies

## References

When you need detailed explanations or examples for specific Clean ABAP rules, consult these reference files:

1. **Complete Guidelines**: Read `references/CleanABAP.md` - the full Clean ABAP style guide with in-depth explanations, rationale, and code examples for all principles
2. **Quick Patterns**: Read `references/quick-reference.md` - condensed good/bad code examples for common patterns
3. **Review Checklist**: Read `references/checklist.md` - a checklist format for systematic code reviews

**When to consult references:**
- To provide accurate citations when explaining violations
- To show users the official reasoning behind recommendations
- To find specific code examples to illustrate points
- To answer follow-up questions about Clean ABAP principles
- When encountering edge cases not covered in the summary above

## Examples

### Example 1: Checking a Simple Method

**Input:**
```abap
METHOD calculate.
  DATA: lv_result TYPE i,
        lv_temp TYPE i.

  lv_temp = iv_value1 + iv_value2.
  IF lv_temp > 100.
    lv_result = lv_temp * 2.
  ELSE.
    lv_result = lv_temp.
  ENDIF.

  ev_result = lv_result.
ENDMETHOD.

出力：

# Clean ABAP Check Results

## Summary
- Total Issues: 4
- Critical: 0
- Major: 2
- Minor: 2

## Major Issues

### Names - Hungarian Notation Used
**Location:** Method calculate
**Problem:** Variables use Hungarian notation prefixes (lv_, iv_, ev_) which adds no value and violates Clean ABAP naming guidelines.
**Recommendation:** Remove prefixes and use descriptive names.

**Anti-pattern:**
```abap
DATA: lv_result TYPE i,
      lv_temp TYPE i.

クリーンなコード：

DATA(result) = value1 + value2.
IF result > 100.
  result = result * 2.
ENDIF.

メソッド - RETURNING ではなく EXPORTING を使用

場所： メソッドシグネチャ 問題： メソッドが RETURNING ではなく EXPORTING パラメーターを使用しているため、関数型の呼び出しスタイルを使用できません。 推奨事項： 出力値が 1 つの場合は、RETURNING パラメーターを使用します。

アンチパターン：

METHOD calculate
  IMPORTING iv_value1 TYPE i
            iv_value2 TYPE i
  EXPORTING ev_result TYPE i.

クリーンなコード：

METHOD calculate
  IMPORTING value1 TYPE i
            value2 TYPE i
  RETURNING VALUE(result) TYPE i.

軽微な問題

変数 - 先行宣言

場所： 2～3 行目 問題： 変数がインラインではなく先行して宣言されているため、宣言と使用箇所の間が離れています。 推奨事項： DATA( ) を使ったインライン宣言を使用します。

言語 - 手続き型スタイル

場所： 5～9 行目 問題： IF-ELSE の代わりに COND を使って条件付き代入を行えます。 推奨事項： 関数型の構文を使用します。

クリーンなコード：

METHOD calculate
  IMPORTING value1 TYPE i
            value2 TYPE i
  RETURNING VALUE(result) TYPE i.

  DATA(sum) = value1 + value2.
  result = COND #( WHEN sum > 100 THEN sum * 2 ELSE sum ).
ENDMETHOD.

総合評価

コードは機能しますが、古い ABAP のパターンを使用しています。主な改善点は、ハンガリアン記法をなくし、EXPORTING ではなく RETURNING を使用し、インライン宣言を優先し、関数型の言語構文を使用することです。これらの変更により、コードは大幅に読みやすく、保守しやすくなります。