Method to get user information (OOP / MVC approach)

Question

I coded a small user script with PHP. I'm improving the script, learning from my mistakes. I recently learned about object oriented programming and wrote 2 big god classes (user class and message class). That didn't feel right, so read about the MVC model. I wrote a controller class to handle the big god classes while being easy to use in the views (I want to be able to rewrite the big god classes without rewriting the views).

This controller class allows me to streamline my big god classes, which is exactly what I wanted to achieve.

I'm replacing the method user_search() with user_search_info(). It should return a row of user information based on the given user_id or user_id. It should return false if the query fails or returns empty. When I am satisfied, I will delete user_search().

The new method works like I want it to, and I find it very elegant and easy to read. But is it the right approach? I'm sure there's still room for improvement.

This is from the user class file:

  class user{
    // new methods don't care about this anymore so this will get deleted
    public $logs = array();

    // ... irrelevant methods left out ...

    // old method
    public function user_search($db_connection, $user_id=null, $username=null){
      $result = false;
      if($user_id == null && $username == null)
        $this->logs[] = "No information to search on has been found.";
      elseif($db_connection == false)
        $this->logs[] = "Something went wrong with the database.";
      else{
        $this->logs[] = "Searching the user in the database.";
        $query_user_search = $db_connection->prepare('SELECT * FROM users WHERE user_username=:user_username OR user_id=:user_id');
        $query_user_search->bindValue(':user_username', $username, PDO::PARAM_STR);
        $query_user_search->bindValue(':user_id', $user_id, PDO::PARAM_STR);
        $query_user_search->execute();
        if($result = $query_user_search->fetch())
          $this->logs[] = "We found the user.";
        else
          $this->logs[] = "We didn't find the user.";
      }
      return $result;
    }

    // new method
    public function user_search_info($db_con, $user_id=null, $user_username=null){
      $query_user_search_info = $db_con->prepare('SELECT * FROM users WHERE user_username=:user_username OR user_id=:user_id');
      $query_user_search_info->bindValue(':user_id', $user_id, PDO::PARAM_STR);
      $query_user_search_info->bindValue(':user_username', $user_username, PDO::PARAM_STR);
      $query_user_search_info->execute();
      return $query_user_search_info->fetch();
    }
  }

And this is from the controller class file:

public function return_user_search_info($search_user_variable, $search_user_value)
{
  $user_search_info = false;
  if($search_user_variable == "user_id")
    $user_search_info = $this->user->user_search_info($this->db_con, $search_user_value, "");
  elseif($search_user_variable == "user_username")
    $user_search_info = $this->user->user_search_info($this->db_con, "", $search_user_value);
  return $user_search_info;
}

Answer 1

What you have isn't really a function to search for a user, but a function to get a user, either by id or by name.

So instead of having one function that does two different things, why not have two functions?

public function getUserById($db_con, $id){
    $queryGetUser = $db_con->prepare('SELECT * FROM users WHERE user_id=:id');
    [...]
}

public function getUserByUsername($db_con, $username){
    $queryGetUser = $db_con->prepare('SELECT * FROM users WHERE user_username=:username');
    [...]
}

Now you don't have to pass empty strings to the function (which is really confusing to use), and the functions have a proper name as well.

Answer 2

Thanks to tim's reply, I was able to create an improved version.

• I split the method in 2 different methods
• I changed the names of the methods to better describe the purpose of the method
• Added bonus: code is easier to read and controller doesn't have to pass empty strings to the model.

Updated methods in the user class (as part of the model):

public function get_user_by_id($db_con, $id){
  $query_get_user = $db_con->prepare('SELECT * FROM users WHERE user_id=:id');
  $query_get_user->bindValue('id', $id, PDO::PARAM_STR);
  $query_get_user->execute();
  return $query_get_user->fetch();
}
public function get_user_by_username($db_con, $username){
  $query_get_user = $db_con->prepare('SELECT * FROM users WHERE user_username=:username');
  $query_get_user->bindValue('username', $username, PDO::PARAM_STR);
  $query_get_user->execute();
  return $query_get_user->fetch();
}

Updated method in the controller class:

public function get_user($variable, $value){
  $user = false;
  if($variable == "id")
    $user = $this->user->get_user_by_id($this->db_con, $value);
  elseif($variable == "username")
    $user = $this->user->get_user_by_username($this->db_con, $value);
  return $user;
}