laravel
2022.09.19
オフショアの品質がやばいのはN+1問題があるからです←こうやって解決する
オフショア関係なく、
事業部側が「制作側にどういう指示を出したらうまく作ってくれるんだろう」って思うことありますよね。
ないなら話が終わってしまうのである設定で行きます。
事業部から制作に依頼があるケースって・・
ぱっと思いつく範囲だと
こんな感じではないかなと思うんですよね
特にb2b2cのメディアやポータルの場合は近いものがあると思います。
このケースには、事業部の依頼の仕方に答えがあります。
この図の中で要件メモと書かせてもらったものが重要で、
本来は画面IDごとに画面定義図があり(ないなら作らないといけないですよ)
その画面定義ごとに用件を伝えていきます(本ブログはその書き方は割愛)
この用件メモをデータフローのCRUDと紐づけるんですね。
そうすると影響範囲も一目瞭然でわかり、
開発者がオフショアだったり、ディレクターが経験不足で全体を見通した用件を言えない場合でも品質の担保や影響範囲を調べることができる強力なソリューションとして使えます。
と、ここまでは事業部でできる精一杯のこと
ここからですが、
開発の範囲で品質を保つためにやらないといけないことがあります。
実際のコードを元にN+1の解消をしながら説明をしましょう。
そもそもN+1コードってわかりますか？
オフショアの代名詞(見つけたら即解雇のクソコード)N+1コードです。
前回のコードの中に堂々とN+1コードがあったため
修正していきます。
そもそもN+1コードとは
リレーション先のデータを持ってくるとき、
そのリレーション元のデータがforeachでN回回って取得されている状態で、
foreachの中でさらにそのリレーションの情報をとるためにsql叩いている状態。
データもってこいという一回とN回回ってる中で実行されるsqlがあるため、
N+1回実行されることからN+1問題と言われます。
10回回すコードなら1回で済むsql文なのに、11回回してしまうクソコードです。
.envに
APP_DEBUG=trueを記載すると
実際に実行されてるsqlがN+1かどうかもわかるので、
確認してみましょう。
というよりも
前回のコードのように、
foreachの中でBelogToのmodelを持ってきてるものは全部N+1なので、
慣れてくるとレビューですぐにわかるようになると思います。。
今は実行すると
$like->jobと実行した時にN+1となるので、
return KeptJob::where('user_id', $userId)->get();
↓
return KeptJob::where('user_id', $userId)->with('job')->get();
で、解消されます。
ただ、jobの先で、jobとnurseryもN+1の関係があるので、
それを解消するために
return KeptJob::where('user_id', $userId)->with('job.nursery')->get();
にするのが正しいです。
さらに、nurseryの先でも
$likeJob->job->nursery->facilityTypes $likeJob->job->nursery->prefecture $likeJob->job->nursery->city
がN+1なので、
return KeptJob::where('user_id', $userId)->with([
'job',
'job.nursery',
'job.nursery.facilityTypes',
'job.nursery.prefecture',
'job.nursery.city'
])->get();
にするのが正しいです。
これで実行するとwhere inの形になってるのがわかります。
これで解消されました。
オフショアに関しては、
pluk使うこと
DRYコードを意識
N+1を使わない
service repogitoryデザインパターンを徹底する
そして前回のenumを使う
これでだいぶ綺麗になるのではないかと思います。
_____
こんにちは突然のだいきです。
オフショアはいいとして
日本人の場合は
下記をベースに基準を設けると良いと思います。
https://github.com/FIELDASIA/gandb_frontend/wiki/Coding-Rules
上記はやろうと思えば誰でもできることなので、上記を実践してないとしたらスキル以前の意識レベルが低いということになると思います
※大前提、TypeScriptが書けない人は「勉強して書けるようになってください」という話なので
いつまで経ってもキャッチアップできない（しない）人はPJに入れる価値が遅かれ早かれなくなると思います
あとはクビほどじゃないにしてもやってない人を優秀とは言えないなレベルのものとして
■ソースコードの責任分離について
①ページ単位で全てのDOMを記述する（コンポーネントを分離してない）
②state管理&ロジックのセットを全てコンポーネント 内に記述してしまう（カスタムhookなどにまとめてない）
・DOMそのものの分離
・DOMとロジックの分離
ここは、この２つを実践しているか？に尽きると思います
■構文について
①特にstate管理の変数に対して、破壊的処理をしてしまう
（非破壊的メソッドやスプレッド構文を用いてコピーを取りながら、state更新をしていくということをしていない）
②公式に非推奨になっていたり、非推奨ではなくても一般的にもう使わない方がいいと言われている構文や書き方、あるいはライブラリを使ってしてしまう
（ReactでClassコンポーネントを使って、componentDidMountを書く
momentJSをいまだに使う
など）
■TypeScriptの型安全について
①TypeScriptで汎用的なメソッドにすべき、同じような処理を個別に実装している
（つまりジェネリクス構文の使い方を理解してない。汎用的なlib関数を書こうとすればジェネリクスを使うしかない）
②同様にTypeScriptでコンポーネント を書く際に汎用的なコンポーネントにまとめるべきところをまとめてない
（同様に見た目だけ統一して、描画する変数の型等だけ動的に変える、のためにはジェネリクスを使うしかない）
③ ①, ②のように汎用的に実装したとしても、安易にany使って実装してしまう
（型を確定できない時に中国メンバーはanyを安易に使ってしまうイメージがあります）
④PickやOmit, Partialといったユーティリティタイプなどを使わないで、とにかく頑張って型定義を列挙してしまう
（anyを使うよりいいですが、保守がキツくなります）
⑤if文を使って型ガードすれば安全にできるところを安易にasで強制書き換えしてしまう。
同様にユーザー定義型ガードなどを使わずasを使ってしまう
⑥新規のプロジェクトなのに、tsConfig.jsonで設定を緩めてしまう
⑦外部ライブラリで、型定義ファイルを別でインストールしないといけないところを、インストールせずanyのまま使ってしまう
⑧APIレスポンスのデータに型を付与しないで、anyのまま扱ってしまう
理想: Promiseオブジェクトに型引数を渡して型を付与する
妥協（APIレスポンスに対してなら許せる）asでアサーションする
⑨コンパイラが型推論できず暗黙anyになっていることに気づかず、そのまま取り回してしまう
（そのケースでは明示的に型アノテーションが必要）
■フレームワークやライブラリについて
①使うべきライブラリを使ってない
（例えばReactでフォームを書くとしたら、react-hook-formを使えばめちゃくちゃ楽にかけますが、使わず頑張ってバグの温床を生む、など）
②VueやReactで実装しているのに、生JSやjQueryでDom操作をすること
（実際に見たことがあるわけではないですが、やっていたらかなりやばいです）
③APIリクエストをaxiosを直接使って書きがちだが、今ならReact QueryやSWRなど、キャッシュやAPI通信のステータス管理（ローディングなど）を便利にラップしてくれるものを使ってない
※100%使わないといけないものではないですが、検討はされているべきかと思います
④状態管理ライブラリを使ってない、もしくはオーバースペックなReduxを使う
（PJ次第なので、React純正のhookやReduxなどをもちろん使ってもいいのですが、一般的にもっと扱いやすい
Jotai, Recoil, valtioなどのライブラリの検討もされているのか？）
⑤グローバル状態管理を適切にしないで、ひたすらpropsのバケツリレーで実装してしまう
（中規模以上OJだとこれは相当生産性下がると思います）
⑤（Next.jsの話）CSSをモジュール化すべきところをしないで、グローバルCSSに書いてしまう
■セキュリティ
①ちょっと細かいですが、APIリクエストのためのアクセストークンをlocalStrageに入れている、という話を聞いたことがあります
（一般には、JSで操作できるlocalStrageはXSSの危険があるのでCookieを使った方がいい、という話をよく見かけます
が、結局一長一短で論争が絶えないようなので一概には言えず、問題というほどではないかもしれないです
ちょっと気になっていたので一応上げておきます）
上記のテキストとリンクには入っていないが、過去のやり取りの中で挙げられた事項も載せておきます
・変数名だけでも、ある程度は型を類推できるようにする。
・APIのレスポンスで、条件によって存在しない値は、nullや空の配列などとして返す。他の条件では存在し得ることが分かる様にして返すということ。型が変わらないのが望ましい。
・query parameterから取得したなどの理由により、本来numberである値がstringになっている場合は、numberに変換して使う。
・2値変数を扱う場合は、0 or 1ではなく、true or falseを使う。
・Hooksのdependencyは全部入れる。中途半端に入れるとバグの原因になり得るので、むしろ使わない方が良い。
汎用的なコンポーネントや関数に、それを使う側の事情による特殊な変更を入れない。必要なら、汎用的な形で変更を入れる。もしくは、ラップする。
・不要なimportを残さない。
・未使用の変数・関数を残さない(後に必要となる可能性を考えて残してあるなら、コメントアウトし、何のために残してあるのかのコメントをつける)。
・console.logは削除するorコメントアウトする。
以上です。
CRUDとコードルールがしっかりしていれば
オフショアでもそれなりにしっかり開発ができます。