hcan Şimdiye kadar incelediğim en temiz projelerden biri. Elinize sağlık. Gördüğüm birkaç şey şunlar:
Eğer kullanıcının giriş yaparak geldiğinden eminseniz $request->user()->id kullanın, emin değilseniz Auth::id() ya da $request->user()?->id. Yoksa Trying to get property... alırsınız.
API için kullandığınız resource controller'lar için Route::resource() yerine Route::apiResource() kullanabilirsiniz. Bu otomatik olarak create ve edit yöntemlerini devre dışı bırakır. Api resource controller php artisan make:controller API/UserController --api şeklinde de oluşturuluyor.
Controller yöntemlerinde dönüş tipine ihtiyacınız yok çünkü onlar direkt tarayıcıya gönderilir. Eğer içeride siz bir controller'ı düz sınıf gibi kullanıp içinden bir yöntem çağırırsanız bu zaten yanlış kullanım demektir. Ayrıca object şeklinde bir dönüş tipi hatanın önüne geçmez, daha spesifik dönüş tipleri belirtmeniz lazım, :User gibi.
return Exception::forgotPasswordException(); şeklinde return yapmanıza gerek yok, Exception::forgotPasswordException(); demeniz yeterli ki aslında throw new ForgotPasswordException() demeniz de yeterli. App\Exceptions\Exception sınıfını açıkçası gereksiz gördüm ve içindeki string ile namespace oluşturma pek sevdiğim bir yol değil.
$user = User::where('email', $request->input('email'))->first(); yerine $user = User::firstWhere(['email' => $request->input('email')]); de kullanılabilir. Sadece bilgi amaçlı yazıyorum.
$FINISHED_VOTE_LIMIT = 12; gibi direkt yöntem içinde tanımlamalar yerine sınıf özelliği olarak tanımlama yaparsanız diğer yöntemlerde de değere ulaşabilirsiniz. Değişmeyecek bir değer ise constant olarak tanımlayın.
->first()->member->id; gibi kullanımlar tehlikeli. Kayıt yoksa Trying to get property... alırsınız. Ya kaydın mutlaka olduğundan emin olun ya da null dönüşe izin veriyorsanız ->first()?->member?->id; kullanın.
!= yerine !==, == yerine === kullanın, tiplerden emin olun.
Observer içinde Job dispatch yapmışsınız. Ben bunu doğru bulmuyorum çünkü event içindeki işlemlerin gerçekleşip de Job'ların ateşlenmesini istemediğiniz durumlar olabilir (örneğin bir artisan komutu içinde yapılan işlemler gibi). Bu durumda event dispatcher'ı da iptal edemezsiniz çünkü bu sefer tüm event içeriğini iptal etmiş olursunuz. Bu sıkıntının kaynağı bir yönteme birden fazla görev atamanız. Job ya da Event'ları sadece istediğiniz yerde anlık kullanın ve/veya listener yazın. Yöntemlere alakasız görevler yüklemeyin, sade olsunlar.
Yöntem isimleriniz güzel, parametre tipleri ve dönüş tipleriniz de var ama aynısı PhpDoc ile de yazmışsınız. Eğer API dokümantasyonu çıkarmayacaksanız gereksiz PhpDoc kullanımlarından kaçının, kod zaten ne yaptığını anlatıyor.
Bazı modellerde accessor tanımlarken eager loading kullanmadan sorgu yapmışsınız. Buralarda dikkatli olmanız lazım, eğer accessor bir iterasyon içinde kullanılırsa n+1 problemine sebep olur. Ör: Vote::getVotedPercentageAttribute() içindeki $this->votedUsers()->count();. Burada şöyle bir durumda var, eager loading kullanırken de bu sefer tüm votedUsers ilişkisini yükler, arada dengeli bir yol bulmanız gerekebilir.
Accessor tanımarken geri dönüş tipine gerek yok çünkü hiç devreye girmiyor. getVotedPercentageAttribute():int değişsiniz mesela, burada dönüş tipi gereksiz.
Model içindeki scope'lar daima Eloquent\Builder dönmeli. Siz normal yöntem gibi kullanıp bool vs döndürmüşsünüz. Ör: Vote::scopeHasUserVoted(). Bu, bir özelliğin suistimal edilmesi demek. hasUserVoted() şeklinde normal yöntem kullanın.
