PHP - Duplicate code or dual use a function for user authentification

All,

I am building a small, non-commercial consumer website. During the 开发者_开发知识库user registry, I want to check whether their chosen user name already exist in my user db (MySQL).

Currently, I re-use the function below, which is mostly used to retrieve a user, based on the user name, during the login process:

function retrieveUserByUserName($userName){
 $dbConnection=$this->dbInstance->createConnexion();
 $query=$dbConnection->prepare("SELECT * FROM users WHERE userName= :userName");
 $query->bindParam(":userName", $userName);
 $query->execute();
 $result=$query->fetchObject('userName');
 if ($result){
  return $result;
 } else {
  return false;
 }
}

This is slightly overengineered just to test whether the user name exists, but my main concern is that it returns the full user object, which might be unsafe and is certainly somewhat wasteful. My alternative would be to create a dedicated function:

function checkUserName($userName){
  $dbConnection=$this->dbInstance->createConnexion();
  $query=$dbConnection->prepare("SELECT * FROM users WHERE userName= :userName");
  $query->bindParam(":userName", $userName);
  $result=$query->execute();
  return $result;
}

This makes the code slightly heavier.

So my question to the SO community is: what is the right tradeoff? Duplicate code or use a function for a secondary purpose? Is there a good rule of thumb for that kind of question, or does it depend?

Many thanks,

JDelage

PS: I'm pretty new at all that, sorry for the very basic question.

PPS: I'm now wondering whether the 2nd query would ever do what I want. If there are no records in the db with those parameters, the query still executes and the value of $result is always true, right?

The main thing is readability. Your code should be readable. And if the function name is retrieveUserByUserName() and you are using it to find if user exists it will make code harder to read. Dedicated function is far better.

Consider this to code samples and choose which one looks better: 1st

if (userExist($username)) 
{  
  register_user(); 
}

2nd;

   $user = getUser($username)
    if (isset($user->id))
    {
      register_user()
    }

if you really want to reuse your code you can put a wrapper function. something like:

function userExist($username)
{
  $user = getUser($username);
  return isset($user->id);
}

It completely depends. I think this is about taste. Programming is all about keeping overview of what you are doing and what you have done. As long as you don't create 20 functions with the same purpose you will be fine.

Most of the time I use a seperate AJAX-request to validate my username when the inputfield loses focus. In that request I use a seperate function which only checks if the username exists in the database.

public static function isUsernameValid($userName){
  $dbConnection=$this->dbInstance->createConnexion();
  $query=$dbConnection->prepare("SELECT count(*) AS total FROM users WHERE userName= :userName");
  $query->bindParam(":userName", $userName);
  $result=$query->execute();

  if($query->fetchColumn() == 0) {
       return true;
  } else {
       return false;
  }
}

In this case you don't retrieve any information about the user itself and don't have to fetch the object. This same function can, and shoud, ofcourse also be used to check once the form is submitted.

Good luck!

I would suggest writing a special function and query for finding out if a username is in use or not, as this is a function which will be called many many more times than your other query which actually wants to retrieve data.
I would also recommend not using "select *" unless you really want to bring the whole object back with you. A query that brings a userid, or just 1/0 is much more efficient.
As rule of thumb - we sometimes are lazy and don't want to write extra code. Too much code also requires a lot more maintenance (should the requirements change, or something like that). On the other hand, we want efficient code.
I would say that in general you want to keep queries for "quality" and queries for "quantity"

• quality = detailed data ("select *")
• quantity = less data per row, but more rows

Hope this helps.