Controller method to store course

Question

I have this controller method to add a course and it works fine but I really don't like this code because I think it's too large and a little bit ugly

I was trying to do it cleaner and a little bit shorter, But don't have knowledge to do that so can anyone please help me with this code?

 public function storeCourse(Request $request)
 {
 $request->validate([
 'slug' => 'required',
 'name' => 'required',
 'image' => 'required',
 'description' => 'required',
 'body' => 'required',
 'audio' => 'required',
 'price' => 'required',
 'level' => 'required',
 'subtitle' => 'required',
 'knowledge' => 'required',
 'required' => 'required',
 'userId' => 'required',
 'categoryId' => 'required'
 ]);
 if ($request->hasFile('image')) {
 $filenameWithExt = $request->file('image')->getClientOriginalName();
 $filename = pathinfo($filenameWithExt, PATHINFO_FILENAME);
 $extension = $request->file('image')->getClientOriginalExtension();
 $fileNameToStore = $filename . '_' . time() . '.' . $extension;
 $request->image->move(public_path('/uploads'), $fileNameToStore);
 } else {
 $fileNameToStore = 'no_img';
 }
 $course = new Course();
 $course->slug = $request->get('slug');
 $course->name = $request->get('name');
 $course->image = $fileNameToStore;
 $course->description = $request->get('description');
 $course->body = $request->get('body');
 $course->audio = $request->get('audio');
 $course->price = $request->get('price');
 $course->level = $request->get('level');
 $course->subtitle = $request->get('subtitle');
 $course->knowledge = $request->get('knowledge');
 $course->required = $request->get('required');
 $course->user_id = $request->get('userId');
 $course->course_category_id = $request->get('categoryId');
 $course->save();
 return redirect()->back();
 }

Answer 1

You could create 2 sub-functions for the different parts in your function.

• validate($request) so that long part is hidden
• getImageName($request)

Also you could remove all those setters for the course and put everything in the constructor:

$r = $request;
$course = new Course($r->get('slug'), $r->get('name'), ...);

• ManaguaMan: Hello thanks for response. Sorry for posting in wrong section. I really don't understand how to do that i understand how to do $course = new Course($r->get('slug'), $r->get('name'), ...); and then $course->save() but really don't understand how to do image upload part and validator part
• Double_A: Create private functions that you can call inside storeCourse(), and then you put the code in those functions.

Answer 2

Avoid redundant code

The fields to be set from the request could be stored in an array - perhaps a class constant:

private const COURSE_ATTRIBUTES = [
 'slug',
 'name',
 'image',
 'description',
 'body',
 'audio',
 'price',
 'level',
 'subtitle',
 'knowledge',
 'required',
 'userId',
 'categoryId'
];

Then those can be used to specify the fields the request should validate with the help of the built-in PHP function array_fill_keys():

$request->validate(array_fill_keys(self::COURSE_ATTRIBUTES, 'required'));

Then when setting the attributes of the new course the array could be used, though special handling of the last two fields would need to be added:

$course = new Course();
foreach (self::COURSE_ATTRIBUTES as $attribute) {
 $course->$attribute = $request->get($attribute);
}
$course->user_id = $request->get('userId');
$course->course_category_id = $request->get('categoryId');

Make a form request class for handling the validation

The validation rules could be moved out to a FormRequest subclass. For example:

namespace App\Http\Requests;
use Illuminate\Foundation\Http\FormRequest;
class StoreCourseRequest extends FormRequest
{
 public function rules()
 {
 return [
 'slug' => 'required',
 'name' => 'required',
 'image' => 'required',
 'description' => 'required',
 'body' => 'required',
 'audio' => 'required',
 'price' => 'required',
 'level' => 'required',
 'subtitle' => 'required',
 'knowledge' => 'required',
 'required' => 'required',
 'userId' => 'required',
 'categoryId' => 'required'
 ];
 }
}

Note that beyond required there are numerous validation rules that could be used - e.g. max, unique, etc.

If the validation fails and the request was an XHR request, an HTTP response with a 422 status code will be returned to the user including a JSON representation of the validation errors..

Then that subclass can be injected instead of Illuminate\Http\Request in the storeCourse method arguments and use $request->all() to get the fields to pass to $course->save().

If the properties that can be set on the course (e.g. slug, name, image, etc.) are declared in the fillable property on the Course model (which could be declared as a public constant that is also assigned to the protected property in the model) then you should be able to simplify the controller method like below (warning - untested):

public function storeCourse(StoreCourseRequest $request)
{
 if ($request->hasFile('image')) {
 $filenameWithExt = $request->file('image')->getClientOriginalName();
 $filename = pathinfo($filenameWithExt, PATHINFO_FILENAME);
 $extension = $request->file('image')->getClientOriginalExtension();
 $fileNameToStore = $filename . '_' . time() . '.' . $extension;
 $request->image->move(public_path('/uploads'), $fileNameToStore);
 }
 $attributes = $request->validated();
 $attributes['user_id'] = $attributes['userId'];
 $attributes['course_category_id'] = $attributes['categoryId'];
 $attributes['image'] = $fileNameToStore ?? 'no_img';
 $course = Course::create($attributes);
 $course->save();
 return redirect()->back();
}