Lookup class to get descriptions of values Following yesterday's advice on my Customer class, I have now created a new class which is for looking up descriptions from a MySQL database. I have a Lookup table which contains type, name, value, and descriptions.

Here's a basic example of type = customer and name = state


Customer States


New Customer
Pending Approval
Approved / Active
Deleted



In this example if I pass in the parameters "customer", "state", and 2, the method will return "Pending Approval".

pdo.inc.php

&lt;?php

// Version 0.1
// Last updated 08 Jun 2016

define('DB_CONFIG_HOST', 'localhost');
define('DB_CONFIG_DB', 'dev');
define('DB_CONFIG_USER', 'dev');
define('DB_CONFIG_PW', 'dev');
$dsn = 'mysql:host=' . DB_CONFIG_HOST . ';dbname=' . DB_CONFIG_DB . ';';
define('DB_CONFIG_DSN', $dsn);

try
{
    $pdo = new PDO(DB_CONFIG_DSN, DB_CONFIG_USER, DB_CONFIG_PW);
}
catch (PDOException $ex)
{
    error_log('Connection failed: ' . $ex-&gt;getMessage());
    die();
}

?&gt;


lookup.class.php

&lt;?php

// Version 0.1
// Last updated 09 Jun 2016

class Lookup
{

    function __construct($pdo)
    {
        $this-&gt;pdo = $pdo;
    }

    function getLookup($lookup_type, $lookup_name, $lookup_value)
    {

        $valid_types = array("customer");
        $valid_names = array("state");

        if (!in_array($lookup_type, $valid_types))
        {
            throw new InvalidArgumentException('Lookup type is not a valid');
        }

        if (!in_array($lookup_name, $valid_names))
        {
            throw new InvalidArgumentException('Lookup name is not a valid');
        }

        if (empty($lookup_value) || !is_int($lookup_value) || $lookup_value &lt; 0)
        {
            throw new InvalidArgumentException('Lookup value is not a valid integer');
        }

        $query = "SELECT lookup_description FROM Lookup WHERE lookup_type = :lookup_type AND lookup_name = :lookup_name AND lookup_value = :lookup_value LIMIT 1";
        try
        {
            $stmt = $this-&gt;pdo-&gt;prepare($query);
            $stmt-&gt;bindParam(':lookup_type', $lookup_type);
            $stmt-&gt;bindParam(':lookup_name', $lookup_name);
            $stmt-&gt;bindParam(':lookup_value', $lookup_value);
            $stmt-&gt;execute();
            if ($stmt-&gt;rowCount() === 0)
            {
                return false;
            }
            else
            {
                return $stmt-&gt;fetchColumn();
            }
        }
        catch (PDOException $ex)
        {
            error_log('Something went wrong in getLookup ' . $ex-&gt;getMessage());
            return null;
        }
    }

}

?&gt;


test.php

&lt;?php

// Version 0.1
// Last updated 09 Jun 2016

include_once('pdo.inc.php');
include_once('lookup.class.php');

try
{
    $l = new Lookup($pdo);
}
catch (Exception $ex)
{
    echo $ex-&gt;getMessage();
    die();
}


try
{
    echo $l-&gt;getLookup("customer", "state", 1); // Returns "New Customer"
    echo $l-&gt;getLookup("customer", "state", 2); // Returns "Pending Approval"
}
catch (Exception $ex)
{
    echo $ex-&gt;getMessage();
    error_log('Customer name lookup failed with: ' . $ex-&gt;getMessage());
}

?&gt;


I'm not sure I like the idea of having fixed arrays for looking up valid parameter values. I was thinking of getting a distinct list from MySQL, but as to do that I might as well just pass the query without validation and let it return null. Any thoughts?
Getting a distinct list from MySQL would slow things down quite a bit, I wouldn't do that. 

Perhaps you're overdoing it a bit? Yes, it is always good to filter user input, but I would do that at the point where this input arrives in your application, not in each class inside the application. You are already binding the parameters, which secures the database against injection. Anything apart from what's in your database will result in false, which might be better behaviour than throwing an exception.

So the protection is in the binding of the parameters, and not really in the validation. 

Talking about validation. There might be a perfectly good reason to insert all that code. Regretably your validation is incomplete. If I give your method a $lookup_value of 10 it will just return false. That might be a real value, or a sign that the value doesn't exist. You would normally use null in these cases, which is what you do when the query fails.

So my suggestions are: 


Filter user input, perhaps with static arrays, directly at the point
of entry. So at the point where you get user input out of $_POST,
$_GET, or anything else. Try to be as strict as possible. When a
value doesn't meet all the criteria, give feedback and allow the user
to enter a correct value. Never let an user freely enter values into
your application in the (false) assumption that the application will
deal with any abnormal values all by itself: It won't. You will
eventually make mistakes.
Keep the validation inside your application very simple and flexible,
this will make it easier to read, modify, and understand the code.
The validation is there to support you to debug your application, it
should not be relied upon as a security feature.
Always bind parameters, of course.


Something like this:

function getLookup($lookup_type,$lookup_name,$lookup_value)
{
  // checking type of arguments
  if (!(is_string($lookup_type) &amp;&amp;
        is_string($lookup_name) &amp;&amp;   
        is_int($lookup_value))) {
    throw new InvalidArgumentException('Invalid arguments in Lookup-&gt;getLookup().');
  }
  // select a description of ???
  $query = 'SELECT lookup_description 
            FROM Lookup 
            WHERE lookup_type  = :lookup_type AND 
                  lookup_name  = :lookup_name AND 
                  lookup_value = :lookup_value 
            LIMIT 1';
  try {
    $statement = $this-&gt;pdo-&gt;prepare($query);
    $statement-&gt;bindParam(':lookup_type', $lookup_type);
    $statement-&gt;bindParam(':lookup_name', $lookup_name);
    $statement-&gt;bindParam(':lookup_value',$lookup_value);
    $statement-&gt;execute();
    // return any results
    if ($statement-&gt;rowCount() &gt; 0) return $statement-&gt;fetchColumn();
  }
  catch (PDOException $exception) {
    error_log('Database error in Lookup-&gt;getLookup(): '.$exception-&gt;getMessage());
  }
  // failure: no results or database error
  return null;
}


While I was writing this code I noticed that you're using strange names in your database: A Lookup table, containing columns like lookup_name. From these names I get no idea what the content of that table is. The names describe the functions in PHP and not their meaning. A database should be comprehensible on its own, without any code operating on it.

PS: Perhaps someone told you to validate the arguments of your method thouroughly. I cannot help that, this happens when many people can give advise.