Laravel controller to manage sent and received messages

Question

I write small inboxing app in Laravel 5.5 for my wesite. In inbox app users can compose new message, read messages and delete messages. I have only one table in my database for inboxes.

Structure of messages table:

And I have model Messages for this table.

Code: InboxController

<?php

namespace App\Http\Controllers;

use App\User;
use App\Message;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Auth;
use Illuminate\Contracts\Encryption\DecryptException;

class InboxController extends Controller
{
    ## Inbox View Control Methods ##

    // View for show new messages of current user

    public function messages()
    {
        $array = array
        (
            'messages'=>$this->user_messages('my_messages'), 
            'count'=>$this->count_messages()['my_messages'],
            'count_sended' => $this->count_messages()['sended_messages'],
        );

        return view('index', $array);
    }

    // View for show sended messages of current user

    public function sended()
    {
        $array = array
        (
            'messages'=>$this->user_messages('my_sended_messages'), 
            'count'=>$this->count_messages()['my_messages'],
            'count_sended' => $this->count_messages()['sended_messages'],
        );
        return view('sended', $array);
    }

    // View for show trashed messages of current user

    public function trashed()
    {
        $array = array
        (
            'messages'=>$this->user_messages('my_sended_messages'), 
            'count'=>$this->count_messages()['my_messages'],
            'count_sended' => $this->count_messages()['sended_messages'],
        );
        return view('sended', $array);
    }

    // View for compose and send new message

    public function compose(Request $request)
    {
        if($request->isMethod('post'))
        {

            // Sended data from html form 
            $to      = $request->to;
            $from    = Auth::user()->id;
            $subject = $request->subject;
            $message = $request->message;

            // Get info about receiver user
            $receiver_user_id = User::where('email', $to)->first()->id;
            $receiver_user_type = User::where('id', $receiver_user_id)->first()->type;  


            // Get info about sender user 
            $sender_user_type = Auth::user()->type;

            // Send message
            Message::create(
                [
                    'subject'=>$subject, 
                    'message'=>$message, 
                    'from'=>$from, 
                    'to'=>$receiver_user_id
                ]
            );

        }

        $count = $this->count_messages()['my_messages'];
        $count_sended = $this->count_messages()['sended_messages'];

        $array = array
        ( 
            'count'   => $count,
            'count_sended' => $count_sended,
        );

        return view('compose', $array);
    }

    // View for read message

    public function message(Request $request, $id)
    {
        try 
        {
            $decrypted = decrypt($id);
        }

        catch (DecryptException $e) 
        {
            return redirect()->back();
        }


        $user_id    = Auth::user()->id;

        $messages   = Message::where('id', decrypt($id))->get();
        dump($messages);
        $message_to_id = $messages->first()->to;
        if($user_id == $message_to_id)
        {
            $subject = $messages->first()->subject;
            $message = $messages->first()->message;
            $status  = $messages->first()->status;
            $date    = $messages->first()->created_at;

            $sender_id = $messages->first()->from;
            $sender = User::where('id', $sender_id)->first()->name;
            $count = $this->count_messages()['my_messages'];
            $count_sended = $this->count_messages()['sended_messages'];

            $array = [
                        'subject' => $subject,
                        'message' => $message,
                        'date'    => $date,
                        'messages'=> $messages,
                        'sender'  => $sender,
                        'status'  => $status,
                        'count'   => $count,
                        'count'   => $count,
                        'count_sended' => $count_sended,
                     ];

            if($status == 0 && $sender_id !== $user_id)
            {
                Message::where('id', decrypt($id))->update(['status' => 1]);
            }

            return view('message', $array);
        }

        else return redirect()->back();

    }

    ## Helper functions for get any info from messages and users table ##

    public function count_messages()
    {
        $user_id = Auth::user()->id;
        $my_messages = Message::where('to', $user_id)->where('status', 0)->count();
        $sended_messages = Message::where('from', $user_id)->count();
        $array = ['my_messages' => $my_messages, 'sended_messages' => $sended_messages];
        return $array;
    }

    public function user_messages($status = 'my_messages')
    {
        $user_id = Auth::user()->id;

        switch ($status) 
        {
            case 'my_messages':
                $messages = Message::where('to', $user_id)->orderBy('created_at','desc')->get();
                break;
            case 'my_sended_messages':
                $messages = Message::where('from', $user_id)->orderBy('created_at','desc')->get();
                break;
            default:
                $messages = false;
                break;
        }

        return $messages;
    }

}

How I can reduce code duplication and beautify my controller code?

Answer 1

Reduce Complexity, Follow SRP
The Single Responsibility Principle states that every module or class should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by the class. All its services should be narrowly aligned with that responsibility.

Robert C. Martin expresses the principle as follows: A class should have only one reason to change.

While this is primarily targeted at classes in object oriented languages it applies to functions and subroutines well.

The public function message(Request $request, $id) function could be broken up into at multiple functions, especially the contents of the if($user_id == $message_to_id) block.

The more separate functions there are the easier it is to understand or read the code. This also makes it easier for any programmer to maintain or debug the code.

Don't Repeat Yourself

In software engineering, don't repeat yourself (DRY) is a principle of software development aimed at reducing repetition of software patterns, replacing them with abstractions; and several copies of the same data, using data normalization to avoid redundancy.

Generally when there is repeating code in a software module it indicates that a function should be written to contain that code or a loop should be written to perform the repetition.

When code repeats in different functions it becomes a maintenance problem. Someone can fix the code in one location and miss it in another location. The solution to this is to write a function for the code that repeats.

Answer 2

• stop using SQL builders in your "controllers"
• stop rendering templates in your "controllers"
• stop repeating authentication in each class method
• stop using static function calls and relaying on global state

Though, I think all those are "laravel best practices" ... hmmm ... I wonder, what conclusions that might lead to.

Answer 3

You can move all your business logic into services. I also recomend you to Use repository pattern in order to reduce number of queries. And finally use dependency injection || Service Container.

Here the link for Service Container tutorial. You can also create seperate class for handling your business logic and then inject them into controllers __constructor. Then you will endup with less than 10 lines of code.