コードアンチパターン

なんでも共通化＋フラグ管理

読みづらすぎ。影響範囲大きすぎ。

$this->consoleLog("ログメッセージ", self::LOG_WARN)

public function consoleLog($message, $mode, $output = true, $display = true) {
switch ($mode) {
 case "WARN":
     $this->log->warning($message)
 break;
 case "INFO":
     $this->log->info($message)
 break;
 }
 if ($display) {
   //おそらく画面上にログを出力する処理
 }
}

スーク

DB共通化

INSERT処理を行いにくい。ALTER処理などの検証をローカルで容易にできない。

DBはひとつで、リモートデスクトップに用意した環境のIPをローカルで接続して使用

スーク

DBを無闇に分けている

発生する問題：難読化。

{
 "metadata" : {
      "title":"タイトル',
      "update_date": "2024/01/14"
 },
 "article" => {
     "content" => 'コンテンツ',
     "signiture" => '署名〜〜〜'
 }
}

といったオブジェクトを階層でテーブル分けしている。
上記の例だと。api_metadataテーブルと、api_articleテーブルに分かれている

SQLの検索時に高速化する意図があるかと思ったが、特にそういった意図はないみたい。
オブジェクトが階層化されているから、テーブルも分けたみたい。。。

？？？？？

おそらくDBを実際に目で確認する実体として考えているみたい？

スーク

マジックナンバー

$config = $this->getConfig(4);

4は、業務ドメインでの取り扱い区分だったらしい。わからん。

設定値関連の補足

$config = array[
 1 => [
  'name' => ***,
 ]
]

この1が区分として管理されていた。場合によってはこれえぐいバグ生みそう

スーク

マジックナンバーに0が入る

nullとemptyと0を取り扱うときに、慎重にならないといけないので嫌。

$config = $this->getConfig(0);

スーク

nullに意味を持たせる（伝聞）

ユーザーから入力された実行回数を保存するカラム「runCount（int）」にて
0は0回
1は1回
…
5は5回以上
という意味で、intを保存する．
nullは未回答という意味になる

0とnullに意味を持たせるのは避ける。今回の例だとintで保存する理由がない

スーク

動かない可能性のあるドキュメントを渡される

渡す時に、動作チェックしてとも言われていない。

ドキュメントの内容を信頼できない。不安定な足場で作業しているようなもの。ストレスが溜まる

スーク

ループの中で不要な情報をログに出力している。
グローバル変数として持っているが、不要。
例えばログに出力する場合、yymmddhisで出力されるため実行時間や経過時間は不要。

スーク

yoda記法
条件説で判定値と期待値が逆転しているやつ。

備考。もしかしてasserEqualsの期待値と判定値が逆転しているのって、之の名残だったりする?

スーク

db::raw()の多様。なんならそこに変数入れているのでsqlインジェクトションされる

スーク

マジックナンバー絡み。

配列のキー（0,1,2）がそのまま種別番号として定義されている。コワスギ！

スーク

オレオレフレームワーク

db::rawやselectの中の関数が共通化されている。パラメータはconst TABLELISTで配列形式で定義されている。
このルールに沿わないものは条件分岐させる必要がある

スーク

foreach (self::TABLELIST as info) {
if(info['joinflg']) {
// こんなのが続く
}
}

スーク

テーブル構造が実装に依存しているパターン

スーク

table名がロジックで固定されている。
以下の場合だと、テーブル名をtable_user_baseとか、table_post_baseとかにしないと使えない

        $datas = DB::table('table_{$base_table_name}_base', 'baseTable')
          ->where('baseTable.id', '=', $id)
          ->get()
          ->toArray();

スーク

if(4 == $data)
のように、ifの中の実測値と変数が逆になっていた。Yoda記法というそうです
PHPStanとかの静的解析ツールを使おう

スーク

意図を判断するのは諦めたけど。

            if( $status == "old"){
              // 2018年以前
              $datas = $datas->where('sub.year','<=', "2018");
            } else
            if( $status == "new"){
              // 2019年以降
              $datas = $datas->where('sub.year','>=', "2019");
            }
          } else {
            if( $status == "old"){
              // 2018年以前
              $datas = $datas->where('main.year','<=', "2018");
            } else
            if( $status == "new"){
              // 2019年以降
              $data = $datas->where('main.year','>=', "2019");
            }
          }

laravelのjoin()やwhere()を条件分岐で付与している

スーク

簡略化。

$tableName = $_GET['tableName']

$query = DB::table($tableName);

//200行くらいある条件分岐も複雑な条件説。

if ($tableName ='post') {
$user_id = 'post_id'
}

//データを取得する
$query->get()->where($columnName, '=', $user_id);

$user_idという変数に別の意味を持たせている。
後続処理をまるっとコピーしてもエラーになるし、条件節が多すぎて、自分の処理に必要なものがわからないためコピれない。デッドラインの兼ね合いと、割れ窓効果でどんどん条件節が肥大化