2

Currently, I am writing a laravel application with a part for sending messages between the staff at the company and the clients.
So, I have a field named "status" in the database. In this field, a value of one indicates that the message is waiting for an answer, a value of two indicates that it has been answered, and a value of three indicates that the message has been closed. There is a problem here, however. It's not clear what these numbers do when someone looks at my code.
Would there be any way for me to define this number or any other way to make my code more readable?
(I'm using laravel eloquent ORM) The code below is for the method that closes a conversation:

    public function close(Request $request)
    {
        $message = Message::find($request->message_id);
//        Status one indicates that a conversation has been closed
        $message->status = 1;
        $message->save();
        return \response($message, 200);
    }
2
  • use constants in the model. if you add your model code and a use case of the status values to your question i might help you. Commented Mar 18, 2022 at 9:23
  • I just added the related method. @N69S Commented Mar 18, 2022 at 9:30

2 Answers 2

0

Use constants in your Message model

class Message
{
    const STATUS_PENDING = 1;
    const STATUS_ANSWERED = 2;
    const STATUS_CLOSED = 3;
//...
}

Then your code will be readable

public function close(Request $request)
{
    $message = Message::find($request->message_id);
    $message->status = Message::STATUS_CLOSED;
    $message->save();
    return \response($message, 200);
}

Or Even better, make it a method in your model on top of the constants values

public function close(Request $request)
{
    $message = Message::find($request->message_id);
    $message->close();
    return \response($message, 200);
}

That way you can in the future upgrade the method, for example

class Message
{
    public function close()
    {
        if ($this->status != self::STATUS_ANSWERED) {
            //log message closed without client answer
        }
        $this->status = STATUS_CLOSED;
        $this->save();
    }
}
Sign up to request clarification or add additional context in comments.

Comments

-1

You could use something like this. First we crate some static variables in the model, representing your conversation status:

public static $_STATUS_PENDING = 1;
public static $_STATUS_ANSWERED = 2;
public static $_STATUS_CLOSED = 3;

When you add those, you will be able to call them statically now:

$message->status = Message::$_STATUS_PENDING; // 1
$message->status = Message::$_STATUS_ANSWERED; // 2
$message->status = Message::$_STATUS_CLOSED; // 3

This is a lot more readable now.

You can go even further with this, and create another array that you will use to display these values, without the need to do if-else statements. Fist we will create a key-value array that will represent the values:

public static function getStatuses()
{
    return [
        self::$_STATUS_PENDING    => 'Pending',
        self::$_STATUS_ANSWERED => 'Answered',
        self::$_STATUS_CLOSED    => 'Closed',
    ];
}

After that, we will create a function that you will be able to call on your Message instance, that will display the status of message:

public function getMessageStatus()
{
    return self::getStatuses()[$this->status];
}

Now, when we have this method, we can simply call it and get the correct status of the message:

$message->getMessageStatus(); //Pending, Answered or Closed

1 Comment

Why declare them as variables ? their values should not be changed.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.