alijumaan / laravel-ecommerce

144 stars 72 forks source link

You may want to use request only instead of assigning another variable #9

Closed elviskc091 closed 3 years ago

elviskc091 commented 3 years ago

https://github.com/alijumaan/Laravel-Ecommerce/blob/4b88d4b63f121e0dc837a8518f39df93f86d22d1/app/Http/Controllers/Backend/CouponsController.php#L33

instead of assigning request parameters to new variable ($data['foo'] = $request->bar) you might want use $request->only('foo','bar')

Coupon::create($request->only('name', 'code', 'type', 'value', 'description'));

This makes your code shorter

alijumaan commented 3 years ago

You are right it's short and more readable.

acomerlatto commented 3 years ago

https://github.com/alijumaan/Laravel-Ecommerce/blob/4b88d4b63f121e0dc837a8518f39df93f86d22d1/app/Http/Controllers/Backend/CouponsController.php#L33

@alijumaan At this point I think you could use the RequestForm validation layer, along with the model fillable property, for security reasons.

Using guarded is not the best pratice for model attributes https://laravel.com/docs/8.x/eloquent#allowing-mass-assignment https://github.com/alijumaan/Laravel-Ecommerce/blob/4b88d4b63f121e0dc837a8518f39df93f86d22d1/app/Models/Coupon.php#L9

Use fillable instead of guarded in Coupon Model https://laravel.com/docs/8.x/eloquent#mass-assignment

protected $fillable = [
    'name', 'code', 'type', 'value', 'description'
];

With the fillable defined and the FormRequest with its validation rules, just use:

Coupon::create($request->validated());

You can remove all of these lines: https://github.com/alijumaan/Laravel-Ecommerce/blob/4b88d4b63f121e0dc837a8518f39df93f86d22d1/app/Http/Controllers/Backend/CouponsController.php#L28-L32