DBからデータを取得するコードに改善の余地があるか知りたい

Question

###改善の余地があるか知りたいコード

class Article extends Model
{

public function countArticleDate()
    {
        // articlesテーブルからcreated_atカラムを指定して全件取得(all()で配列化)
        $created_at_columns = $this->select('created_at')->get()->all();

        // 取得した件数分、繰り返し処理でフォーマットを変更、空の配列に格納
        $count = count($created_at_columns);
        for ($i = 0; $i < $count; $i++) {
            $created_at[] = $created_at_columns[$i]->created_at->format('Y_m_d');
        }
        // 重複しているデータ(日付)を除外
       $unique_created_at = array_unique($created_at);
       // 重複なしのデータ(日付)を数える
       $count_created_at = count($unique_created_at);
       // 合計値をコントローラーに渡す
       return $count_created_at;
    }
}

###articlesテーブルの構造とcreated_atカラムの情報
articlesテーブル

id	user_id	body	created_at	updated_at
1	1	テスト	2021-05-18 11:02:38	2021-05-18 11:02:38

 `created_at` timestamp NULL DEFAULT NULL,

###取得結果(dd()で確認)

created_atカラムの日付データのフォーマット変更時と変更後、重複している日付データの削除後の合計値をdd()で確認

//フォーマットの変更成功
array:2 [▼
  0 => "2021_05_18"
  1 => "2021_05_22"
]

//count()で合計値の取得成功
2

上記のメソッドは、articlesテーブルからcreated_atカラムの日付データを日付重複なしで取得後、created_atカラム(日付データ)の合計値を取得したいと思い、日付データの重複をなくす為に、繰り返し処理で時刻を削除したフォーマット(年/月/日の状態)に変更した後にarray_unique()で重複を削除、そしてcount()で合計値を取得すると考え作成しました。

最終目的としましては、重複なしの日付単位で数えた投稿データの合計値をユーザーの投稿累計日数として表示したいと考えております。

例えば、ログインしているユーザーが、2021-05-18、2021-05-19、2021-05-20という日付データを保持している投稿を各日付ごとに2つずつ投稿していると仮定します。

このような場合、articlesテーブルでは、2021-05-18、2021-05-19、2021-05-20の投稿データが各2つずつ存在しているのでデータ数としては6つあることになると思います。

重複なしの日付単位で考えるとユーザーが投稿しているデータは3つになると思います。この重複なしの日付単位で数えた投稿データ数をユーザーの累計投稿日数として扱いたく、累計投稿日数3日という数をarticlesテーブルから取得したいと思い、上記のようなコードを作成致しました。

投稿している日数の合計値を表示したいユーザーとリレーションしているarticlesテーブルのcreated_atカラムを、一旦全て取得してその件数分の繰り返し処理で、取得したcreated_atカラムの日付データのフォーマットを変更していることが非常に効率が悪いと思っています。私自身の力ではこのコードに対する改善策が思いつきませんでした。

上記コードの改善点、間違い等のアドバイスをいただきたく存じます。

何卒、よろしくお願いします。

###追記
回答者様から、上記のコードは全て間違っているとのご指摘を受けました。
私、独学で学習しておりまして、学習してきたことが正しくないと言われどの箇所が間違っているのか分からない状態です。
指摘を真摯に受け止め、基礎固めからやり直したいと思っておりますので、もし可能であれば少し具体的に間違いをご指摘いただきたく存じます。

何卒、よろしくお願い致します。

###開発環境
laravel 6.20.16
php 7.4.15
MySQL 5.7

Comments

[phper.k]

countArticleDate() がどのクラスに描かれたものなのか、Controller ? Model ?
そういう情報なしにアドバイスは困難ですよね？

[yuina711]

申し訳ございませんでした。
修正しておきます。

[phper.k]

あと、最終的に何をしたくて、countArticleDate() が必要なのでしょうか？
XY問題含んでいる質問に見受けられます。
https://ja.wikipedia.org/wiki/XY%E5%95%8F%E9%A1%8C

[yuina711]

再度、質問内容の不備をご指摘くださりありがとうございます。

質問内容を修正いたしました。
お手数をおかけして申し訳ございませんでした。

[phper.k]

> リレーションしているユーザーの投稿している日数の合計値として使用したいと考えております。

ログインしているユーザーの投稿の日付ごとの投稿数を表示したいということ？
日付の合計っておかしいと思うので。

[yuina711]

分かりにくい文章で申し訳ございません。

> ログインしているユーザーの投稿の日付ごとの投稿数を表示したいということ？

重複なしの日付単位で数えた投稿データの合計値をユーザーの累計投稿日数として表示したいと考えております。

例えば、ログインしているユーザーが、2021-05-18、2021-05-19、2021-05-20という日付データを保持している投稿を各日付ごとに2つずつ投稿していると仮定します。

このような場合、articlesテーブルでは、2021-05-18、2021-05-19、2021-05-20の投稿データが各2つずつ存在しているのでデータ数としては6つあることになると思います。

重複なしの日付単位で考えるとユーザーが投稿しているデータは3つになると思います。この重複なしの日付単位で数えた投稿データ数をユーザーの累計投稿日数として扱いたく、この累計投稿日数3日という数をarticlesテーブルから取得したいと思い、上記のようなコードを作成致しました。

再度、説明にご不明な点がございましたら、何なりとおっしゃってください。
何度もやりとりしてくださりありがとうございます。

Answer 1

それぞれ、わからないところはリファレンスを確認するべし。

routing

Route::resource('articles', 'ArticleController');

migration

<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

class CreateArticlesTable extends Migration
{
    /**
     * Run the migrations.
     *
     * @return void
     */
    public function up()
    {
        Schema::create('articles', function (Blueprint $table) {
            $table->bigIncrements('id');
            $table->unsignedBigInteger('user_id');
            $table->string('body');
            $table->timestamps();
        });
    }

    /**
     * Reverse the migrations.
     *
     * @return void
     */
    public function down()
    {
        Schema::dropIfExists('articles');
    }
}

factory

<?php

/** @var \Illuminate\Database\Eloquent\Factory $factory */

use App\Article;
use Faker\Generator as Faker;

$factory->define(Article::class, function (Faker $faker) {
    return [
        'user_id' => function () {
            return \App\User::query()->inRandomOrder()->first()->id;
        },
        'body' => $faker->sentence
    ];
});

seeder

<?php

use Illuminate\Database\Seeder;

class UserSeeder extends Seeder
{
    /**
     * Run the database seeds.
     *
     * @return void
     */
    public function run()
    {
        factory(\App\User::class, 10)->create()->each(function (\App\User $user) {
            $user->articles()->saveMany(
                factory(\App\Article::class, 100)->make()
            );
        });
    }
}

model

<?php

namespace App;

use Carbon\Traits\Timestamp;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\BelongsTo;

class Article extends Model
{
    use Timestamp;

    /**
     * @var string[]
     */
    protected $fillable = [
        'user_id',
        'body'
    ];

    protected $appends = [
        'created_date'
    ];

    /**
     * @return string
     */
    public function getCreatedDateAttribute(): string
    {
        return $this->created_at->format('Y-m-d');
    }

    /**
     * @return BelongsTo
     */
    public function user(): BelongsTo
    {
        return $this->belongsTo(User::class);
    }
}

<?php

namespace App;

use Illuminate\Database\Eloquent\Relations\HasMany;
use Illuminate\Foundation\Auth\User as Authenticatable;
use Illuminate\Notifications\Notifiable;

class User extends Authenticatable
{
    use Notifiable;

    /**
     * The attributes that are mass assignable.
     *
     * @var array
     */
    protected $fillable = [
        'name', 'email', 'password',
    ];

    /**
     * The attributes that should be hidden for arrays.
     *
     * @var array
     */
    protected $hidden = [
        'password', 'remember_token',
    ];

    /**
     * The attributes that should be cast to native types.
     *
     * @var array
     */
    protected $casts = [
        'email_verified_at' => 'datetime',
    ];

    /**
     * @return HasMany
     */
    public function articles(): HasMany
    {
        return $this->hasMany(Article::class);
    }
}

controller

<?php

namespace App\Http\Controllers;

use Illuminate\Contracts\View\View;
use Illuminate\Http\Request;

class ArticleController extends Controller
{
    /**
     * Display a listing of the resource.
     *
     * @return View
     */
    public function index(Request $request)
    {
        // ログインしているユーザーの投稿を全て取得
        $articles = $request->user()->articles;

        // 記事を投稿した日数を累計
        $groupedArticlesByDateCount = $articles->groupBy('created_date')->count();

        return view('articles.index', compact('articles', 'groupedArticlesByDateCount'));
    }
}

Comments

[yuina711]

phper.k様
ご回答ありがとうございます。

提示していただきましたコードを参考に、再度コードの書き方等の基礎的な部分から見直したいと思います。

何度もやりとりしてくださり、本当にありがとうございました。

[phper.k]

> 再度コードの書き方等の基礎的な部分から見直したいと思います。

コードの書き方以前に、「Laravel の設計思想」を理解することの方が先ですね。
「Laravel の設計思想」を理解するには、MVCの基礎概念の理解が必要です。
https://ja.wikipedia.org/wiki/Model_View_Controller

テイラーさんがいうには、Laravel は （厳密には）MVC ではないといいますが、MVCを理解していない人がLaravelを使っても、カオスになるだけ。

[yuina711]

学習の方向性までご教授してくださり、ありがとうございます。

下記リンク先を参考にしつつMVCを意識してコードを書いていたつもりなのですが、全く理解できていない自分に気づくことができました。もう一度、MVCの基礎概念を踏まえた上でLaravelの設計思想についても見直したいと思います。
https://zenn.dev/mpyw/articles/ce7d09eb6d8117

独学で学習していると、phper.k様のように間違い等を指摘してくださる方が周りにいないのでとても助かりました。本当にありがとうございました。

[phper.k]

mpyw さんの記事は上級者向けの記事です。
Laravel が用意している基本機能をフル活用できた上で、さらに上の人が対象です。

あなたがやるべきことは、この「公式ドキュメントの日本語訳」を網羅して読むことですね。
https://readouble.com/laravel/6.x/ja/installation.html

[yuina711]

ご返信頂きありがとうございます。

私は読むべき情報源を誤っていたのですね。
まずはドキュメントを繰り返し読むことから徹底したいと思います。

再度やるべきことをご教授してくださり、ありがとうございました。

Answer 2

改善も何も全部間違ってるので
Laravelは投げ捨てて、PHPとデータベースとSQLの基礎から勉強しないと無理。

Comments

[yuina711]

kawax様
ご指摘くださりありがとうございます。

再度勉強をやり直したいので、もし可能であればもう少し具体的に間違っている箇所を教えていただけると嬉しく思います。
よろしくお願い致します。