AIMSで指摘事項として挙がるパターンについて、ドキュメントを一通り確認し、コメントしていく。
対象は2024/10/5時点のパターンを解説する公式のドキュメント。意外と長くなったので、2つに分割した。この記事はその最初の方。
AIMSのチェックより前の段階で、ODC Studio上の警告は全部潰す、Remove unused elementsを定期的に実行する、というのは基本動作にしてほしいところ。
参考
基本的な使い方
ドキュメント
- Maintainability findings - ODC Documentation(保守性)
- Performance findings - ODC Documentation(性能)
- Security findings - ODC Documentation(セキュリティ)
保守性カテゴリのパターン
感想
全体的にプログラム中に残るゴミを消せ、というパターンが多い感触。
Long undocumented flow (長いロジックにコメントがついていない)
以下の条件を満たすと発生する。
- Action Flowの要素数がClientは20以上、それ以外は40以上
- Commentが配置されていない
長いロジックは読みにくいので、まあ、妥当な印象。
Missing descriptions on public element or parameter
以下のもののDescriptionが空だと発生。
- App/Library
- Publicな要素
- Publicな要素のInput/Output Parameter
Descriptionが無いと確かに保守時に困るのでこれも妥当。
以下の例外ケースは指摘対象外になるらしい
- Entity/StructureのAttribute
- 慣習的に用途が明らかになる名前がついている場合(よくあるEntityの共通列であるCreatedByなどが対応)
この例外ケースについては、どれが該当するのかを明示してほしいのと、できたらプロジェクト・契約単位とかで変更できるようにしてほしいところ。
Hard-coded statements(固定の分岐条件、かつ決して到達しない分岐がある場合)
IfのConditionにTrueなどを設定し、決して到達しないブランチがある場合。プログラムを追っていると、明らかに無駄な分岐はよく見るので、そういう場合の対策になる。
Unused action in app or library (使われていないActionがある)
これはどちらかというとODC Studioの警告時点で、開発者自身で気づいて消しておいてほしいところ。
Unused Aggregate or SQL Query (使われていないAggregate/SQLがある)
同上。
Reminder comments (Action Flowに配置したCommentでIs Reminder=Yesのものが残っている)
Is Reminder=YesのCommentは、後で対応しようと思ったことを備忘録的に残しておけるオプション。
つまり、これが残っているということは消し忘れなので、ちゃんと対応するのが妥当。
これもODC Studioに警告として出てくるので、AIMS以前の段階で、各開発者にきちんと対応しておいてほしい。
Too much disabled code (Action Flow中にDisable Elementした要素が多すぎる)
見にくいのは確か。
OutSystemsは、一般的なプログラミング言語ほどバージョン管理が強くないので、迂闊に一部を消すと戻すのが面倒だが……、まあ、仕方ないか。
性能カテゴリのパターン
感想
基本的にO11にもあったもの。Unlimited records in SQL queryのようにバックエンドがAmazon Aurora PostgreSQLに変わったことで、 RDBの違いによる影響は発生しているようだ。
Aggregate or SQL query inside a cycle (ループ内のAggregate・SQL)
N+1問題。
ループ対象のクエリと、ループ内のクエリを結合するというのが一般的な解法。
親のデータセットが小さい時や、ループ内で複雑なロジック結果を検索条件に使いたいときは、許容できるかもしれない。
Inefficient query count (SQLのCountプロパティを使っている)
OutSystems 11のベストプラクティスにもあったので詳しめの解説はそっちを読んだほうがいいかもしれない。
Countプロパティにアクセスすると、本体と同じ条件のクエリを投げてその件数を取得する。本体は計算が入っていたり不要な列の取得が入っていたりして効率が悪い。解決策は、件数取得専用の別のクエリを作成し、その結果を使う。
Inline CSS style (画面要素自体にCSSが設定されている)
CSSの配置場所としては、まずはStyle Guide (実体はTheme)。次に、画面やBlock専用のスタイルであればそれぞれの編集画面で「CSSアイコン」をクリックして開くダイアログで編集する。
性能観点だと、OutSystemsで作るアプリケーションに、小さなCSSファイルの読み込み増を気にするようなものがあるだろうか、という疑問はある。保守性の観点では、避けたほうが良いと思う。
Long server requests timeout (サーバー側の処理に設定したTimeoutが長すぎる)
Server Request Timeoutがデフォルトの10秒より長いと発生する。
(ちなみにODCでは上限は60秒でそれより長くすることはできないようだ)
長いTimeoutはサーバー側のリソースを食うのと、ユーザー側の待ちが発生する(基本的に非同期に動くのでそんなに気になるかは疑問だが)のは確か。よって、設計や実装を工夫して短縮したり、サーバー側処理を非同期にうつしてクライアントから一定間隔でチェックするのが望ましいのも確か。
そうした設計・実装には担当者のスキルや時間が必要なので、場合によっては負の影響を覚悟の上で、長めに設定するのはありうる。
例えば、上限20秒が想定されるサーバー処理を、工数に余裕のない状況で非同期に移したりするか? というのは疑問。確認の上、Dismissedに設定される事は多そう。
Multiple server requests (Aggregates or Actions) inside Client Actions (1つのClient Action中で複数のサーバー処理を呼んでいる)
サーバー側処理は通信を伴うので、そのオーバーヘッドを避けるため、1つのClient Actionから呼び出すサーバー処理は1つにまとめる、という指摘。
レビューしているとこの指摘がされそうな実装を頻繁に見る。特にTraditional Webからプログラムを移行したりすると多くなるようだ。
Sequence of connected Aggregates (Aggregateが連続しており、後のAggregateが前のを参照している)
DBサーバーとの余分な通信のオーバーヘッドを気にしている。1つのAggregateにまとめよう、という指摘。
一般的にはこの指摘の通り。取得するデータが少なくて、分割したほうが読みやすくなるような場合は、そんなに目くじら立てなくても良いかな。
Unlimited records in Aggregate (AggregateのMax. Recordsが未設定)
この指摘は、よく嫌がられる。
「このクエリは、さほどレコード数が多くないマスタを読んでいるだけなので、どう運用されても過大になる事はないし、仕様としての上限はない」、という場合にも指摘として挙がってしまうので。
回答としては、ベストプラクティスに従うと、「決して到達することのない上限値」をセットしておいて、ということになる。個人的には、Dismissedにするのも面倒なので、そもそも指摘にあげないオプションが欲しい。
Unlimited records in SQL query (SQL要素で、取得件数を絞り込む条件が書かれていない)
Aggregateの場合と似ているが、SQLのMax. Recordsは実はDBカラ取得件数を絞り込んでくれない。
よって、SQL要素に書くSQL文のなかで取得件数を絞り込んでおく必要がある。
なお、2024/10/5時点のドキュメントには以下のように書かれているが、ODCの内部DBはPostgreSQLであるため、O11の時のドキュメントをコピーしてきて直し忘れていると思う。
Use ROWNUM (for Oracle) or TOP (for MS SQL Server) in the SQL query to limit the number of records to the required amount. Note that in SQL queries,
ODC Portalから「How to resolve」ボタンで表示した説明文は以下のように正しそうな記述になっていた。
Use LIMIT (Postgres) in the SQL query to restrict the number of records to the required amount.