Is this an good OOP Design in MVC PHP for getting User Details? I was given a API project by my co-worker to work on half created a system in slim PHP. The project consists of the Models and controllers. When the route is called it calls the Controller with the necessary function in the Controller, then query data from the database in Model functions and get back the response.

I want to know is this the correct way to implement OOP in PHP API applications? If not what are the changes and components I have to do to make it OOP MVC app?

User Controller -> UserController.php

public function getUser($request, $response, $args)
{
    $userId= $request-&gt;getAttribute('id');

    $user = $this-&gt;User-&gt;getUserById($userId);

    if (!$user) {
        return $response-&gt;withJSON([
          "error" =&gt; true, 
          "message" =&gt; "cannot get user info", 
          "data" =&gt; null
        ]));
    }

    return $response-&gt;withJSON([
        "error" =&gt; false, 
        "message" =&gt; "cannot get user info"
        "data" =&gt; $user
    ]);
}


User Model -> User.php

public function getUserById($userId)
{
    $sql = "SELECT id, name, email
            FROM users
            WHERE id= :id";

    try {
        $stmt = $this-&gt;db-&gt;prepare($sql);
        $result = $stmt-&gt;execute([
            'id' =&gt; $userId
        ]);
        if ($result &amp;&amp; $stmt-&gt;rowCount() == 1) {
            return $stmt-&gt;fetchAll()[0];
        } else {
            return false;
        }
    } catch (\PDOException $e) {
        $this-&gt;logger-&gt;info("cannot get user by id from DB " . $e);
        return false;
    }
}
There is not much OOP to review it. Actually that's just two functions, so one can review only the code, not the object structure. All in all there is a lot of repetitive or outright useless code

public function getUser($request, $response, $args)
{
    $userId= $request-&gt;getAttribute('id');
    $user = $this-&gt;User-&gt;getUserById($userId);
    return $response-&gt;withJSON([
        "error" =&gt; !$user, 
        "message" =&gt; $user ? "success" : "no such user"
        "data" =&gt; $user ?: null
    ]);
}


as you can see, there could be just one return statement

public function getUserById($userId)
{
    $sql = "SELECT id, name, email FROM users WHERE id= :id";
    $stmt = $this-&gt;db-&gt;prepare($sql);
    $result = $stmt-&gt;execute(['id' =&gt; $userId]);
    return $stmt-&gt;fetch();
}


I took out all the useless code, namely


it is useless to verify $result. In case of error an exception will be thrown
it is always useless to check the rowCount()
why use fetchAll() if you need only one row?
it is useless to add any condition because fetch() will return FALSE already if no record found.
the same goes for the try-catch. There should be a site-wide error handler that will log the error and do something else like show a generic 500 error in the browser