やめた方が良いコーディング・設計 アンチパターン

やめた方が良いコーディング・設計 アンチパターン:

レビュー時にアーとなる、お話にならない設計をまとめていきたい。

自分のためというより、話が通じないレベルの設計を言語化したい。

言語関係なく、WEBサービスを作る時の、コーディング・設計の基本アンチパターン集としてまとめたい。

こんなこと書く人いないと思うレベルの悪質度の高いもの、極力避けてほしい書き方・設計を、気づいたらここに記述していく。

とくに、個人的には、HTMLとDBの章を読んでほしい。


超基本


条件分岐が多い、階層が多い、循環的複雑度が高い

メソッド自体の責務が多い、無意味な分岐が多い場合に、循環的複雑度が跳ね上がることが多い。

分岐が多い、循環的複雑度が多いということは、考慮しなければいけない条件が多いわけで、当然バグを埋め込みやすくなる。大罪である。

典型的なif文の分岐、ネストに関しては下記ページが詳しく書かれているため、一読する事をオススメする。
https://qiita.com/pakkun/items/9bef9132f168ba0befd7


インデントを揃えない

エディタやIDEの自動整形を使ってほしい。もしくは、コミット時に自動で整形するツールを導入してほしい。

言語によって異なるので割愛するが、最低限インデントはきちんと整えましょう。

ってか、せめて最低限そのくらいやってからレビュー出してほしい・・・


不要な再帰処理

WEBサービスに再帰処理が必要な理由が全く理解できない。というか、ほぼいらないと思っている。

JavaScriptで再帰処理が必要なほど数学ロジックが求められるようなサービスを作っていなければ、ハノイの塔を解くアルゴリズムやソート処理でも作っていなければ、99%はその再帰処理は普通のコーディングで十分表現できる。

不要な再帰処理は大罪である。


共通化・抽象化


コードを共通化せずにコピペする

同じような処理が別に必要になったときに、共通化、module化、カプセル化などの修正作業を行わず、単純にソースをコピペする人が世の中には存在する。

大多数の人は出会うまではありえないと思うだろうがこれが本当にいるのだ。マジキチである。

保守する人の気持ち、後に改修する人の気持ちを考えてほしい。

「リリースを早くするためにコーディングのスピードを上げる」などという言い訳は通用しない。

その愚行により、その後にその機能の追加改修コストが膨れ上がるのだ。それをイメージしてほしい。想像してほしい。今後なにが起きるかを考えてほしい。第三者が改修するときに困り、苦しむことを想像してほしい。

あなたのその行為は愚行であり、システムに毒を埋め込む行為だという事を自覚してほしい。

大罪どころか死刑である。万死に値する。


module化、カプセル化しない、意味のあるメソッド切り分けをしないスパゲッティコード

ちゃんと仕様ごとに意味のあるClass分け、module分けをしてほしい。

数百行のメソッドを作らないで欲しい。あとで読む人のことを、あとで修正する人のことを考えてほしい。

第三者が読むときにどう理解するのか、第三者が拡張するときにどう修正するのかを想像してほしい。大罪である。


抽象化を全くしない、もしくは極端に抽象化しすぎる

ケースバイケースなので全パターンに当てはまらないが、抽象化を全くしないということは普通のコーディングではありえない。

必ずシステムを開発中には何かしらの共通化・抽象化が発生するものであり、そこを回避することはまず不可能である。抽象化すべきところは抽象化しましょう。

逆に抽象化を極端にしすぎるケースも多々あるが、コストパフォーマンスの問題と可読性の問題、影響範囲が広がる問題が存在する。

極端な抽象化と共通化は、修正時の影響範囲の拡大も伴う。あまりに巨大な共通化はバージョニングの概念も設けたほうが良い。

どちらも難しい問題であるが、「なにも考えない」事は罪である。ましてやコピペは決して許されない。



IntやBoolなどを使わず、Stringで全て表現しようとする

プログラミングというものを一から勉強し直したほうが良い。

それが数字なのか、それがtrue/falseの2つから表現されるものなのか というのは非常に重要な情報で、バグの回避としてプログラマにもコンパイラにも非常に有効で有用な情報である。

それをあえて回避するのが全く理解が出来ない。

型で表現できるものは型を使いましょう。なぜ型という概念があるのかを理解してほしい。いうまでもなく大罪である。


連想配列やMapを使い、String -> String で全てを表現しようとする

意図しないバグが起きてもコンパイルでエラー検知が出来ない、何が保存されているのか可視化されないのが罪。

第三者が見たときに、リファクタしようとしたときに、仕様を追加しようとしたときに影響範囲が不明となりやすい。

コンパイル言語であれば、普通に変数で定義すれば良い。バグがあったらコンパイルエラーで気づける。

コンパイル言語でClassやStructを使わず全てを連想配列やMapで表現する意義が一体何があるのか?デメリットしか無い。大罪である。


変数


同じ意味、同じIDを指す2つ以上の変数名をシステム内に作る

同じ意味を指すものは常にシステム間で1つの名前で表現してほしい。

例えば、 video_idvidsmall_id なるものが全部同じIDを示していたことがあった。

1つにしてほしい。なぜ2種類以上の表現を使うのか?

第三者が見たときにそれが違うものを指すと思う事を想像してほしい。そしてやめてほしい。罪である。


変数のスコープを考えない、不必要に広くする

不必要なグローバル変数を使うのは禁止。他の変数も極力スコープは最小限にしてほしい。

変数のスコープを狭くすることは、後の改修やリファクタで非常に有効であるし、コードの可読性も非常に良くなる。

ゆえに不必要にスコープを広くする事、スコープを考えないことは罪である。


変数に再代入する

定数で良いものは定数を使う。一度定義した変数に代入する行為は可読性を下げ、バグを生みやすくする。難しい言葉を使うならば、参照透過性を壊すのだ。

大多数の変数は、(例えばJavaScriptであればconstなどの)定数で定義でき、再代入をしなくても良い。状態変化をすることは最小限にすべきであり、不要な状態変化は罪である。
https://qiita.com/jwhaco/items/6851a0a88a35c5a54fc0


例外


例外処理をロジックとして扱う

例えば、「データが存在しないときに例外を投げる、それを一つ上の階層でtry catchで取得し、別のデータで補完して、その後の処理を続ける」などが該当する。

例外を投げずにそのまま別のデフォルトデータを入れて返せばいい。いったいなぜ例外を投げて、使用側という上の階層で処理する必要があるのか?

「本当の例外」ではない、想像できるロジックや機構、エラーにならないものを例外として飛ばしてはいけない。

例外というのはプログラムのメインロジックを中断する事故が発生するようなものに利用してほしい。goto文のように使うのは大罪である。


例外を握りつぶす

errorログに出力せずに例外を握りつぶす事、無かった事にするのはやめよう。バグが起きた時、問題が起きた時、調査のときに非常に困る。数年後に第三者が困ることを想像してほしい。

Golang などでエラーを _ で握りつぶすケースはよく見るが、そこでもしも問題が起きてしまった時、調査を行うときにどうするのか?

例外とはイレギュラーなもので、エラーとはイレギュラーなものが原則である。基本的にログに出力してほしい。無かった事にしないでほしい。

もしも無かった事にしていいのであれば、それは例外を使うのがおかしく、エラーにする事がおかしい設計である。例外を握りつぶして無かった事にするのは大罪である。


HTML


DBやKVSにhtml自体を文字列として保存する

DOM構造を変更したい時、デザインを変更したい時、CSS(class)を変更したい時 などにいったいどうするつもりなのか。

テンプレートという便利なものがあるのに、それを使わない理由はなぜなのか。

第三者が修正したいときにどうするつもりなのか。

DBやKVSにHTML構造を保存するやつはクビにしたほうが良い。それ自体が大罪である。


JavaScriptソース内に<div>要素などを書き、HTMLに挿入して組み立てる

HTMLの表現はHTML上で作るべきだし、世の中にはテンプレートエンジンというものがある。JavaScriptのソース内に直でHTML構造を残すべきではない。

理由は最終的にHTMLで出力されるものが非常に分かりにくくなるからである。

例えば、 $('#id').prepend('<div>insert</div>'); のようにdiv要素を埋め込むべきではない。
<div>insert</div> はHTMLソース上に存在すべきである。JavaScriptのソース上に書かなくても、classがあればJavaScript上で取得や操作、指定場所への出力は可能である。
hidden をつければHTML上から隠す事もできる。全く問題はない。

JavaScriptのソース内に直でHTML要素をそのまま書く必要などどこにあるのか?エディタやIDEの支援機能も使えなくなるし、JavaScriptとHTMLという異なる概念の切り分けも出来なくなる。

とはいえ、一行くらいなら別に良いと思う(コストとのトレードオフも考慮して)。でもそれを複数行以上やるのは上記理由で大罪である。


APIのレスポンスにHTMLを返す

なぜJsonを使わないのか?と思う。JavaScriptでJsonで取得し、それをHTML上で置き換えることがそんなに工数がかかることなのだろうか?

こちらも、最終的にHTMLで出力されるものが非常に分かりにくくなり、エディタやIDEの支援機能も使えなくなるパターンが非常に多い。

APIでHTMLを返すということは、そのHTMLのベースはバックエンドのソースコード内にString形式で記述されている。いったいテンプレートとはなんなのか?を考えさせられる愚行である。

将来そこに追加改修をする第三者の気持ちを考えてほしい。HTMLがちゃんとHTMLとして壊れていないか?を考慮するために、毎回APIを実行しなければいけなくなり、さらに全てStringで書かれてるためにIDEの支援も受けられないのである。

さらに、APIのレスポンスが返らない事にはHTMLデータ自体が存在しないために、そのDOM要素を使うJavaScriptコードの処理も遅延を考慮して開発しなければいけなくなる。コード設計以前に、システムの設計として難易度が跳ね上がるのだ。

将来、改修する時の追加改修コストが膨れ上がること、修正する第三者が苦しむ気持ちを考えてほしい。大罪である。


DB


外部キーを使わない

このくらい良いだろうと思って、やりがちである。

が、考えてほしい。外部キーを使わないということは全てのコードにそのIDが存在する事を確認するためのバリデーションが必要になる。その大変さを本当に理解しているのか?と。

もしもIDが存在しないデータをInsertしてしまうバグを第三者が後の拡張修正で入れた時にどうするつもりなのか? そのテーブルを設計したあなたに問題があるのです。

DBを分けなければいけなったり、パーティショニングが必要になるなど、外部キーを使えないやむを得ないケースは存在する。

だが、意図せずサボって外部キーを使わないことは、後にコーディングを大変にする要素と、原因特定が困難なバグを生み出す可能性を増やす罪である。


null default nullのカラムを乱用する

データが存在しない可能性のあるカラムは極力最小限にしたほうが良い。

大多数のnull default nullのカラムを大量に使うテーブルは、master table、relation table、transaction table のように分類することが出来るであろう。

データが存在しないnull の可能性があるカラムを常に考慮することはコーディングで想像以上に大変である。ゆえに極力最小限にすべきである。

データ設計は一度失敗すると後からリカバリすることは非常に難しい。なにも考えず1つのテーブルに全データをぶち込むのは愚行であり、大罪である。


JSON型を乱用する

クエリの更新、取得の難易度が跳ね上がる、カラムの型でデータの規制ができなくなる、indexが効かないなど無数に問題が存在する。

JSON型は最後の手段として使うべきで、99%の問題では使うべきではない。

そもそも普通にテーブルとカラムを定義するではダメなのか?本当にJSON型じゃなければいけないのか?あなたのそれは手抜きではないのか?

安易にJSON型を使うのは大罪である。


マイクロサービス


ローカルで開発できない

マイクロサービス化しすぎたために「dev環境にいちいちアップできないと、実行も確認も出来ない」という開発スタイルが存在する。

ありえないだろう。なんのためのマイクロサービス化だ。システムのスケーリングも重要だが、第一目的は開発しやすくすることではないのか?

「障害が起きたときに影響範囲を最小限にする」事のみ重視して、ローカルで開発できなくする事など愚行である。必ずバグを生む。

「dev環境のマイクロサービスをローカルで扱えない」と「ローカル環境にマイクロサービスを立ち上げて開発できない」の両方が起き、毎回dev環境にデプロイしないと開発できないマイクロサービスなどやめたほうが良い。

ローカルで開発できないという行為は、大罪を超えて、万死に値する。


マイクロサービス間で、同じDB・KVSのテーブルデータを扱う

「マイクロサービスAでinsertしたデータを、マイクロサービスBでupdateする」などが該当する。

なんのためにマイクロサービスにしたのか全く理解できない。

マイクロサービスごとに、使用するDB・KVSのテーブルデータを分けないと、互いにマイクロサービス間で影響しあい、各マイクロサービスが独立しなくなるのではないか?

互いに干渉しあうそのサービスはマイクロサービスといえるのだろうか?ただ実行システムを分けただけのものをマイクロサービスなどと言わないでほしい。

データレイヤ、インフラストラクチャレイヤのレベルで完全に分離し、データの流れはAPIのみでやり取りするというのが基本である。

複数のマイクロサービスで、同じDB・KVSのデータを更新・参照し、その状態変化で互いに影響しあうのは、大罪である。というか、それはマイクロサービスとは呼ばない事に気づいてほしい。


マイクロサービス間の障害影響範囲が大きい

「マイクロサービスAが死ぬと、マイクロサービスBに影響し、マイクロサービスCに影響し、マイクロサービスDに影響し、マイクロサービスEに影響し、サービスFに影響してエラーが起きる」

みたいなケースが存在する。マイクロサービスのメリットの影響範囲の切り分けが全く効いていない。むしろデメリットしか残っていない。

こんな状態ならモノリシックに戻したほうがいい。大罪である。


リリース


リリース時期を言い訳にして、設計力の低いコードをマージしようとする

問題のある設計・コードのまま、リリースするとその後に工数が数倍に膨れ上がることを想像してほしい。

良い設計でも時が経つとコードは腐りやすいし、完璧な設計などなかなか出来ない。これは万人が悩み続ける問題である。しかし、明らかに有害な設計・コードは、その腐る時間が数倍短くなる。あなたはシステムに毒を埋め込んでいるのだ。時限爆弾を埋め込んでいるのだということを自覚してほしい。

リリース時期が決まってても、良い設計は出来る。リリース時期のせいではない。あなたの設計力とスケージュール能力に問題があるのです。

そのままリリースするとその有害で毒のある設計・コードのせいで第三者が苦しむことを想像して下さい。その問題を第三者に押し付けないで下さい。

それをわかった上で、その時限爆弾を埋め込みつつリリースするかどうかを、上長や担当者と話し合い決めるべきです。

今修正すれば1日で終わるものでも1年後に修正すると1ヶ月かかるかもしれません。それを想像して下さい。大罪です。

まだまだあると思うが、一旦このへんで。(思い出したら追記していく

コメント

このブログの人気の投稿

投稿時間:2021-06-17 05:05:34 RSSフィード2021-06-17 05:00 分まとめ(1274件)

投稿時間:2021-06-20 02:06:12 RSSフィード2021-06-20 02:00 分まとめ(3871件)

投稿時間:2020-12-01 09:41:49 RSSフィード2020-12-01 09:00 分まとめ(69件)