レガシーなゴッドクラスのリファクタリング(PHP)

エキサイトの山口です。 業務にて大規模なリファクタリングを行いましたので、その経験からどのように行ったのかを書かせていただきます。

リファクタリング

  • 10ページ以上のページが1つのページ用のクラスに集約していた。
  • .htaccessで他ページとして表示していた。
  • コードが複雑化してしまい、手を入れにくい状態になっていた。

なぜこうなったかという経緯(想像)

  • 集約させたページは全て基本同じ構成のリストページ。
  • 複数種類のリストページを単一の検索ページのロジックで担っていた。
  • 一元化することで同じ内容のコードを書かなくて良くなる。
  • そのため全て集約化していったようだった。

結果起きたこと

  • 同じ構成ではあるが、1ページごとにやはり違う部分があり、その違う部分のfunctionが増加すればするほど複雑化が進み、コードの量が増せば増すほど読みにくさが上がってしまった。

感想

正直、すっごく読みにくい!!!!
そして、頑張ってちょっと読むとバグがあったりする。でもいっぱいコードがあるから全然見当たらない!
一元化のメリットはありますが、保守しづらいので、リファクタリング必須な状況に陥ってしまいました。

思い切ってリファクタリングした理由

SEO改善のためページをいじらなくてはいけなくなったけれど、集約しすぎていてすごくやりにくくなってしまった。
工数はかかるが、剥がした方がこの先のSEO改善がすごくやりやすくなると判断したため行いました。

リファクタリングで行ったこと

  • まずはとにかく使っているclassやfunctionを読む。
  • 今回のページでは、表示コンテンツ自体は一元化がほぼできていたのだけれど、とにかくとにかくHTMLのメタ要素の出しわけが各ページで分かれていて、HTMLのメタ要素のfunctionが大量発生するという酷い状況だったため、そこを注意して行うようにした。

  • 同じ構成のリストページのため、共通して使うUtil Classを作成

  • 各ページ用classを作成。ページ依存のfunctionはここに入れる。
  • HTMLのメタ用の共通classを作成。
  • function名は一目で理解できる名前に変更。リファクタリングではfunctionやclassが既に作成された状態から始めるので、名前が昔の命名に引きづられがち。理解しやすい命名に変更することで中身が変わらなくてもわかりやすさが段違いに変わる。
  • 変数名もわかりにくいものは一目で何が入っているかわかりやすく変更。
  • 一つのページで全て行なっていたので、クエリパラメータが全ページ共通。そのためクエリパラメータ用のclassを作成して、他ページでも簡単に使用できるようにした。
  • 複雑化に拍車をかけていたif文の嵐を綺麗に整頓。elseやelseifを使わないようにするだけで大分読みやすさが変わる。
  • functionを別classに入れ直すだけでなく、内容も精査、整頓。既に使われていないfunctionを削除したり、bugfixを行ったり、共通化できるところは全て共通化してコード数を減らすことに注力。
  • 共通の表示コンテンツの表示数などは共通classに定数を置き、それを使用するようにした。
  • パンくず作成用classやHTMLのメタキーワード作成用classを新規作成。今までは変数にそのままざっくり入れて表示していたので、classを使用しわかりやすく使いやすくした。
  • 工数的に全てのページを一気に引き剥がすことができなかったので、各ページ用classを先に作成。特に他ページと違う点が多いページから引き剥がすようにして、簡単に引き剥がしができるようにした。(現在はほぼ引き剥がし済み) 

以下が構成のbefore afterのイメージ図になります

before

after

行った感想・注意点

結構大規模な改善なので、チームメンバーとのすり合わせのためWIPで作業しました。最初のプルリクでざっくり作った時、命名が悪すぎて全てのclass名が変更になったときは名前の勉強が必要だなと思いました。。

JetBeans製品を使っていても気軽にfunction名やclass名を変えると、漏れが出てしまうことがあるので、そこが注意するポイントかなと思います。
一つのページ用のクラスで10ページ以上のページを切り替えていると、そのページ用のクラス独自の共通機能が存在してくるので、その共通機能の切り出しが肝だなと感じました。

一つのページ用のクラスで複数のページを管理していた時のメリットを消さずに別ページに分けるには、共通クラスが重要です。込み入った機能を別ページ用のクラスでも簡単に使えるようにすれば、同じページ用のクラスで管理する必要はなくなります。
バグったりしたら嫌だな、触りたくないなと思っているコードがあったとしても、とにかく臆さずに頑張ってリファクタリングをすると、その後の改善がとても楽になります。

なかなか工数が取りにくく、結果もわかってもらいずらい上に大変だったりしますが、一度行うととても楽にその後実装できるので、ビジネス側的にもメリットはかなり高いです。
エンジニアの気持ちの盛り上がりも結構違います。そして、かなりフットワーク軽く改善などを行えますので、その後絶対に良い効果があります。
表面的にはわかりづらいですが、とても効果があることです。ぜひ行いましょう。