-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
モデルでインスタンスとして多用される配列を、配列としてもアクセスできる構造体的なRowクラスへの変更 #84
Comments
@uzulla |
@fc2dev |
uzulla
changed the title
提案:モデルでインスタンスとして多用される配列を、配列としてもアクセスできる構造体的なRowクラスへの変更
モデルでインスタンスとして多用される配列を、配列としてもアクセスできる構造体的なRowクラスへの変更
Aug 18, 2020
uzulla
added a commit
to uzulla/fc2blog
that referenced
this issue
May 19, 2021
User、PasswordResetToken、EmailLoginTokenにて実装をしてみた。 |
Merged
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
現状基本的にBlogsModelなどから取得する「インスタンス」はfetchした行なので、記述されているデータが自明でないことが多く、見通しがよく有りません。
例として、以下は「ランダムに1件ブログを取得する」メソッドですが、この返り値は
blogs
テーブルの1行配列ですが、コードからどのようなカラムがあるのかは類推できません。blog/app/src/Model/BlogsModel.php
Lines 215 to 224 in 22e993c
一般的に、カラムに対応するプロパティをクラスとして書き換えることで見通しがよくなります。
ただ、既存コードが配列に強く依存しているため、全体の書き換えはかなりの工数がかかります。
提案
ArrayInterface、およびIteratableを実装した配列としてアクセス可能なクラスを実装し、各種返り値を可能なタイミングでそれに差し替え、順次型をつけて見通しがよくするステップをふむことで、コードを長期間フリーズせずに順次移行ができるかなと考えております。
移行中はArrayとRowクラス混在してしまうのは難点ですが、とりあえずその場合でもArrayとしてあつかって書いておいてもらえれば動くには動くのと、現状のテーブル数は13なので作るだけ作るのは可能と考えております。
とりあえずビジネスロジックはかえず、要所にアノテーションを追記する形で静的解析をきかせてバグを減らし、開発の省力化ができます。
なお、典型的には
Model
という命名が多いですが、これはすでにつかわれているので、Model\Row\BlogRow
などの命名になる想定です。改修後は
現状、暗黙になっている以下のようなコードが
blog/app/src/Model/BlogsModel.php
Lines 354 to 363 in 2c92beb
などと型付けができるようになることを想定しています。
一部はそのまま配列にする予定です
たとえば、以下のようなものは当座そのまま配列にしておく予定です。(データ量が大きくなるかもしれないですし、しぼりこんでいるものを広げると、なにか悪影響があるかもしれませんので)
blog/app/src/Model/BlogsModel.php
Lines 182 to 193 in 2c92beb
The text was updated successfully, but these errors were encountered: