Controller in Laravel with parsing, creation and relationships

Question

The following code represents the store method of a resource controller. A user can create an training offer on my website and can pass various information like website, trainers, date, title, address, videos and also upload a coverImage and multiple images for a gallery.

The following steps are conducted:

• Validate the input in StoreTraining
• Parse the input from user in ParseStoreTraining
• Create the training model in TrainingRepository
• Upload cover image in TrainingRepository
• Upload gallery in MediaRepository
• Create relationships for training model (this is a hasManyThrough relation for gallery and video) in MediaRepository.

Here comes the store method from the TrainingsController.

public function store(StoreTraining $request)
{
    \App\Repositories\TrainingRepository::create($request);

    return view('home.training.show');
}

The StoreTraining is a FormRequest that handles the validation:

class StoreTraining extends FormRequest
{
    /**
     * Determine if the user is authorized to make this request.
     *
     * @return bool
     */
    public function authorize()
    {
        return true;
    }

    /**
     * Get the validation rules that apply to the request.
     *
     * @return array
     */
    public function rules()
    {
        return [
            'addressid' => 'required',
            'trainer'   => 'required',
            'dates'     => 'required',
            'mode'      => 'required',
        ];
    }
}

The TrainingRepository class handles the creation of the training.

class TrainingRepository
{
    public static function create(Request $request)
    {
        $parsedRequest  = new ParseStoreTraining($request);

        $profileTraining = ProfileCalender::create($parsedRequest->all());

        // Upload Image 
        if (!empty($request->file('coverImage'))) {
            $path = Storage::disk('public')->putFile('trainings/' . $profileTraining->id, $request->file('coverImage'));

            $profileTraining->cover_image = $path;
            $profileTraining->save();
        }

        // Update relations
        $mediaRepository = new MediaRepository($profileTraining);

        if (!empty($request->gallery)) {
            $mediaRepository->addGalleryToTraining($request->gallery);
        }

        if (!empty($request->video_url)) {
            $mediaRepository->addVideo($request->video_url);
        }
    }
}

The ParseStoreTraining request converts the input to be useable for the ProfileCalender::create method:

class ParseStoreTraining
{
    private $request;

    private $trainerA;

    private $trainerB;

    private $institute_id;

    private $beginn_date;

    public function __construct(FormRequest $request)
    {
        $this->request = $request;

        $this->identifyTrainer();

        $this->identifyInstitute();

        $this->identifyDate();
    }

    public function all(): array
    {
        return [
            'idAddress'   => $this->request->addressid,
            'trainerA'    => $this->trainerA,
            'trainerB'    => $this->trainerB,
            'beginnDate'  => $this->beginn_date,
            'mode_id'     => $this->request->mode,
            'idInstitute' => $this->institute_id,
            'viewable'    => false,
            'idOrga'      => getOrga(),
            'title'       => $this->request->title,
            'coreArea'    => $this->request->focus,
            'url'         => $this->request->website,
            'price'       => $this->request->price,
            'description' => $this->request->description,
        ];
    }

    private function identifyTrainer(): void
    {
        if (is_array($this->request->trainer)) {
            $this->trainerA = $this->request->trainer[0];
            $this->trainerB = $this->request->trainer[1] ?? null;
            return;
        }

        $this->trainerA = $this->request->trainer;
        $this->trainerB = null;
    }

    private function identifyInstitute(): void
    {
        $this->institute_id  = \Auth::user()->getInstituteId();

        if (empty($this->institute_id)) {
            throw new \Exception('User has no related institute id', 1);
        }
    }

    private function identifyDate(): void
    {
        // Convert JS Date to MYSQL Timestamp
        $cleanDate = preg_replace('/\(.*\)/', '', $this->request->dates);
        $beginn_date = strtotime($cleanDate);
        $this->beginn_date =  date('Y-m-d', $beginn_date);
    }
}

A training has many galleries and videos through a media class. The MediaRepository is responsible for creating these relations:

class MediaRepository
{
    private $training;

    public function __construct(ProfileCalender $training)
    {
        $this->training = $training;
    }

    public function addGalleryToTraining(array $images): void
    {
        if (empty($images)) {
            return;
        }

        $media = $this->training->media()->create([
            'institute' => false, // prevent duplicate images if user does not understand how to use gallery
        ]);

        $gallery = $media->galleries()->create();

        foreach ($images as $image) {
            $image_path = Storage::disk('public')->putFile('trainings/gallery/' . $gallery->id . '/pictures', $image);
            $avatar = \Image::make($image->getRealPath());

            $avatar->fit(221, 146);
            $avatar_path  = 'trainings/gallery/' . $gallery->id . '/avatars/' . $image->hashName();
            Storage::disk('public')->put($avatar_path, $avatar);

            $gallery->images()->create([
                'path_image' => $image_path,
                'path_avatar' => $avatar_path,
            ]);
        }
    }

    public function addVideo(String $url, String $name = null): void
    {
        $media = $this->training->media()->create([
            'institute' => false, // prevent duplicate images if user does not understand how to use gallery
        ]);

        $media->videos()->create([
            'url' => $url,
            'name' => $name,
        ]);
    }
}

Any suggestions how I could improve the code?

Answer 1

There are some great things you're doing in this code.

1. Not placing logic into your controller
2. Using repositories to control your data
3. Using requests to validate data in

Some things that could be improved.

1. Repositories are tightly coupled
2. store method on the controller is tightly coupled to a repository
3. Lack of interfaces controlling repositories

I'll step through all the areas that could be improved to add some context.

When you explicitly create an instance of a class within in a method of another class, you are binding the instantiation of not just the required class instance but the specific class itself, this is always true when you call static methods on classes, as such I'd recommend that you don't use static methods at all.

It's much more desirable in most cases to loosly couple class instances and Laravel provides an easy way to do this through its IoC implementation or App instance.

The IoC doesn't explicitly require an interface but it is preferable to make it easier on you as a developer, it also allows you to achieve a better design overall by ensuring that some object oriented design principles are adhered to. The go to design I use is SOLID. SOLID outlines 5 rules to keep your code consise and maintainable. In this case by using the IoC we are enforcing (L) Liskovs substitution principle, (I) Interface segragation, and (D) Dependency Inversion.

An example.

// app\Repositories\TrainingRepositoryInterface

interface TrainingRepositoryInterface {
    // No longer a static function
    public function create(Request $request)
}

// app\Repositories\TrainingRepository

class TrainingRepository implements TrainingRepositoryInterface {
    public function create(Request $request) {
        //
    }
}

With an interface we can bind it to an instance in a provider. We now have interface segregation.

// AppProvider.php (register function only)

public function register() {
    $this->app->bind(TrainingRepositoryInterface::class, TrainingRepository::class);
}

We now have Liskovs substitution principle in place, we can use this to swap out TrainingRepository for another repository that has the same interface if we want to.

In your controller we can complete this implementation by injecting the interface now it is registered.

// app\Http\Controllers\TrainingsController

class TrainingsController extends Controller {
    protected $trainingRepository;
    
    public function constructor(TrainingRepositoryInterface $trainingRepo) {
        $this->trainingRepository = $trainingRepo;
    }

    public function store(Request $request) {
        $this->trainingRepository->create($request);
        return view('home.training.show')
    }
}

Repeat the process with your other repositories:

1. Create interfaces
2. Bind interfaces to concrete classes in a provider
3. Inject these repositories in the constructor of your training repository using the interfaces instead of manually instantiating them.

You now have a de-coupled implementation with interface segregation, an inverted dependency model and all your instances are substitutable by switching out the instances in the IoC.

I hope this helps.