0

※初めに断っておきます。
※これは侍エンジニアのテラコヤというサービスで匙を投げられたものです。
※別手法での実装を勧められましたが、なぜこのようなことが起こるのか分かっているに越したことはないと思い、投稿させていただきます。
※急ぎではないので、お時間のある時に答えていただけると幸いです。

laravelによる勤怠管理システムを開発しています。
個人勤務一覧のボタンを押すと、初めのページでログインしているユーザーのこれまでの勤務の全てを表示し、さらにそのページで日にちの検索を行うと日にちで絞り込んだ勤務時間の確認ができるようにしたいです。
個人勤務の一覧を表示するための処理で、for文が意図しない回数まわり、
勤務日で絞り込んで検索しているにも関わらず、(初めに訪れた時と同様に)これまでの全ての勤務が表示されてしまいます。

helpers.php

日にちによる絞り込みの検索を行うことで、引数の$dateにstringの日にちが入り、結果として$dateListにその日の日にちの情報のみ入るようになっています。for文はそれをカウントして、日数分回すようになっています。

if (! function_exists('searchAttePsn')) {
/**
 * ログインしている人の勤怠記録を表示する
 *
 * @param  object $user  string $date
 */
function searchAttePsn($user, $date = 'all')
{
    //現在ログインしている人のidでモデルからこれまでの勤怠記録(休憩除く)を全て取得
    $psnId = Auth::id();
    $psnAtte = Attendance::where('id_u', $psnId)->get();

    //日にちで検索した場合は、引数で与えられた日にちで絞り込み
    if (!($date === 'all')) {
        $psnAtte = $psnAtte->where('date', $date);
    }

    //日ごとに出勤・退勤のレコードがあるので、それらをまとめて出勤日のカラムの値を順に引き出し
    $dateList = $psnAtte->unique('date')->pluck('date');
    $dataSet = collect([]);

    //for文を回す回数
    $forNum = (count($dateList));
    //$forNumの値は意図通り1
    // ddd($forNum);

    //これまでの勤務記録があれば、for文内で分類、計算
    if ($psnAtte->isNotEmpty()) {
        for ($i = 0; $i < $forNum; $i++) {
            //出勤時間のカラムから値を順に引き出し,その値でCarbonインスタンス生成
            $psnStrt = new Carbon($psnAtte->whereNotNull('start_working')->pluck('start_working')->get($i));
            //退勤時間のカラムから値を順に引き出し,その値でCarbonインスタンス生成
            //勤務中の人の為に、①で条件を分岐
            $getTimeValue = $psnAtte->whereNotNull('end_working')->pluck('end_working')->get($i);
            $psnEnd = new Carbon($getTimeValue);
            //出勤日のカラムから値を順に引き出し,その値でCarbonインスタンス生成
            $psnDate = new Carbon($dateList[$i]);
            //出勤時間と退勤時間の差で勤務時間を計算
            $workTime = $psnStrt->diff($psnEnd);

            // ①退勤時間の値があれば退勤時間と勤務時間を記録
            if(isset($getTimeValue)) {
                $psnEnd = $psnEnd->format('H:i:s');
                $workTime = $workTime->format('%H:%I:%S');
            } else {
            //退勤時間の値が無い(勤務中)なら、それぞれ以下のように記録
                $psnEnd = '---';
                $workTime = $workTime->format('%H:%I:%S') . '勤務中';
            }
            //一日の出勤日、出勤時間、退勤時間、勤務時間のリストをコレクションにする。
            $dailyData = collect(['idlist_a' => $dateList[$i], 'start_work' => $psnStrt->format('H:i:s'), 'end_work' => $psnEnd, 'work_time' => $workTime]);
            //コレクションを配列に加えていく。
            $dataSet[$i] = $dailyData;
        }
    } else {
            $dataSet = null;
    }
    return $dataSet;
}

試したこと

ddd()を使って様々値を確認しました。
ddd(count($dateList))で$dateListのカウント数も見てみましたが、意図通り1回でした。
しかしfor文内で$j++、if($j>1){ddd($j);}等してみると複数回,回っていることが確認されました。

declare(strict_types=1);
とストリクトモードというものを試しましたが、関係ないようでした。

githubのURLを貼っておきます。
https://github.com/akirasasakiatgithub/attendance

3 件の回答 3

1

https://github.com/akirasasakiatgithub/attendance/blob/31b2ed1dbff2bb7a178ce737b00a09281f123bf1/app/Http/Controllers/AttendanceController.php#L90-L91
日付を指定しても(日付指定の処理が行われた後に)指定していないのと同様の処理が呼び出されてしまってますね、これが原因です。

コードを整理すると良いかもしれません。例えば今回は、
https://readouble.com/laravel/9.x/ja/queries.html#conditional-clauses
この辺りの機能を使うと「日付が指定された場合」「指定されていない場合」とで処理を分ける必要がなくなり呼び出す側、呼び出される側、共に見通しが良くなるのと、それ以前に今回の意図しないような呼び出し自体が発生しなくなります。

1
  • こんなに早く回答していただけると思っていませんでした。 最近、凝集度という概念を知り、その観点からも全体の設計をやり直した方がいいなと思っていました。 その点でも、とても有益な情報でした。ありがとうございました。
    – orange
    Commented 2022年7月18日 8:19
0

直接の回答ではありませんが,もう少し踏み込んでデバッグする根気が必要だと思います。 dd() ddd() は実行をそこで止めてしまうので,もう少し工夫が必要です。

【超入門】Laravelのデバッグ手法22選 - Qiita

xdebug でブレークポイント実行するのが王道ですが,少し導入の敷居が高い部分があるので,もう少しカジュアルな方法を紹介します。上記で紹介されている laravel-dump-server を使う方法です。これをコマンドラインで立ち上げていると, dump() 関数の出力先が,ブラウザに返す HTML ではなくコマンドを実行したコンソール上に変化するため,実行を止めずに継続する場合には特に確認しやすく萎える恩恵があるでしょう。これを使う上で,方針を示すと

  • 実行を止めずに継続的に監視する
  • デバッグ関数を差し込む箇所を増やす

これらを両方実践するのが最も有効な手法だと思われます。

dump(['line' => __LINE__, 'variables' => compact('i', 'forNum')]);

これを疑わしい場所に埋めまくって,ループカウンタに影響を及ぼしていそうな場所を特定する作業を行ってください。

但し,記載された関数の中では特に怪しい場所はないように思えました。予想ですが,なにか別のところで勘違いをしている可能性を疑っています。

3
  • 呼び出し側の問題でしたね。ちなみにこれにダンプで気づくには「ループ外に書いた処理が2回ダンプされている」でいけたかも
    – mpyw
    Commented 2022年7月18日 0:34
  • こんなに早く回答を頂けるとは思っていませんでした。 とても参考になる情報ありがとうございます。 非常に助かりました。
    – orange
    Commented 2022年7月18日 8:17
  • 1
    「ループ外に書いた処理が2回ダンプされている」の意味が分かっていなかったのですが、今になってfor文内のデバッグの値は2度目に呼び出された際の値だったということに気づき意味が分かりました。ご教示いただいた方法だとこのようなことにもすぐに気づけるということですね。
    – orange
    Commented 2022年7月18日 10:47
0

for文内ではiを利用してますが、
コメントでjでddしていると記載されているので、何か勘違いがあるのでは?と思いました。

この質問に回答するには、ログインする必要があります。

求めていた回答ではありませんか? のタグが付いた他の質問を参照する。