PDO wrapper with validation

Question

After moving from mysql_ to PDO I created a PDO handling function. Basically this function takes parameters, checks them and returns end result. The thing is that I created this ~1 year ago and haven't really optimized it and after looking at it again I wanted to see if I can improve it somehow, but right now I can't really think of anything. So I came here.

Note: Code is long, but after writing hundreds of MySQL request it saves some space.

function UltimatePDO($Type, $DB, $SeWhere = "", $SePointer = "", $Values = "", $Param = "", $UpPointer = "", $Extra = "")
{
    global $dbh;
    global $abc;

    if(empty($DB) || empty($Type))
    {
        PDOError("Not all parameters");

        return false;
    }

    if($Type == "INSERT")
    {
        if(!empty($Values) && !empty($Param))
        {
            $Lenght = count($Values);
            $Lenght1 = count($Param);

            if($Lenght == $Lenght1)
            {   
                $a = 1;
                $Values[0] = Replace($Values[0]);
                $Param[0] = Replace($Param[0]);
                $Rez = array(":$Values[0]" => "$Param[0]");
                $Rez1 = array();
                $Rez1 = array_merge($Rez1, $Rez);
                $New = ":$Values[0]";
                $In = implode(",", $Values);

                while($a < $Lenght)
                {
                    $Values[$a] = Replace($Values[$a]);
                    $Param[$a] = Replace($Param[$a]);
                    $New .= ",:$Values[$a]";
                    $Rez = array(":$Values[$a]" => "$Param[$a]");
                    $Rez1 = array_merge($Rez1, $Rez);

                    $a++;
                }

                try
                {
                    $stmt = $dbh->prepare("INSERT INTO $DB ($In) VALUES ($New)");   
                    $stmt->execute($Rez1);
                    $Affected = $stmt->rowCount();

                    if($Affected)
                    {
                        return true;
                    }
                    else
                    {
                        PDOError("Can't insert into DB");

                        return false;
                    }
                }
                catch (PDOException $e) 
                {
                    PDOError($e->getMessage());

                    return false;
                }
            }
            else
            {
                PDOError("Not all parameters");

                return false;
            }
        }
        else
        {
            PDOError("Not all parameters");

            return false;
        }
    }

    if($Type == "SELECT" || $Type == "COUNT")
    {
        if(!empty($SeWhere) && !empty($SePointer))
        {
            try
            {
                if($Type == "SELECT")
                {
                    $stmt = $dbh->prepare("SELECT * FROM $DB WHERE $SeWhere = :user $Extra");
                    $stmt->execute(array(":user" => $SePointer));

                    return $stmt->fetch(PDO::FETCH_ASSOC);
                }

                if($Type == "COUNT")
                {
                    $stmt = $dbh->prepare("SELECT COUNT(*) FROM $DB WHERE $SeWhere = :user");
                    $stmt->execute(array(":user" => $SePointer));
                    $count = $stmt->fetch(PDO::FETCH_NUM);

                    return $count[0];
                }
            }
            catch (PDOException $e) 
            {
                PDOError($e->getMessage());

                return false;
            }
        }
        else
        {
            PDOError("Not all parameters");

            return false;
        }
    }

    if($Type == "SELECT NO PARAM" || $Type == "COUNT NO PARAM")
    {
        try
        {   
            if($Type == "SELECT NO PARAM")
            {
                $stmt = $dbh->prepare("SELECT * FROM $DB $Extra");
                $stmt->execute();

                return $stmt->fetch(PDO::FETCH_ASSOC);
            }

            if($Type == "COUNT NO PARAM")
            {
                $stmt = $dbh->prepare("SELECT COUNT(*) FROM $DB $Extra");
                $stmt->execute();
                $Re = $stmt->fetch(PDO::FETCH_NUM);

                return $Re[0];
            }


        }
        catch (PDOException $e) 
        {
            PDOError($e->getMessage());

            return false;
        }
    }

    if($Type == "SELECT TABLE" || $Type == "SELECT TABLE NO PARAM")
    {
        if($Type == "SELECT TABLE")
        {
            if(!empty($SeWhere) && !empty($SePointer))
            {
                try
                {
                    $stmt = $dbh->prepare("SELECT * FROM $DB WHERE $SeWhere = :user $Extra");
                    $stmt->execute(array(":user" => $SePointer));

                    return $stmt->fetchAll(PDO::FETCH_ASSOC);
                }
                catch (PDOException $e) 
                {
                    PDOError($e->getMessage());

                    return false;
                }
            }
            else
            {
                PDOError("Not all parameters");

                return false;
            }
        }

        if($Type == "SELECT TABLE NO PARAM")
        {
            try
            {
                $stmt = $dbh->prepare("SELECT * FROM $DB $Extra");
                $stmt->execute();

                return $stmt->fetchAll(PDO::FETCH_ASSOC);
            }
            catch (PDOException $e) 
            {
                PDOError($e->getMessage());

                return false;
            }
        }
    }

    if($Type == "UPDATE")
    {

        if(!empty($Values) && !empty($Param) && !empty($UpPointer))
        {
            $Lenght = count($Values);
            $Lenght1 = count($Param);

            if($Lenght == $Lenght1)
            {
                $Set = array();
                $Rez = array();
                $Values[0] = Replace($Values[0]);
                $Param[0] = Replace($Param[0]);
                $First = array("$Values[0] = :$Values[0]");
                $First1 = array(":$Values[0]" => "$Param[0]");
                $Rez = array_merge($Rez, $First);
                $Set = array_merge($Set, $First1);
                $MakeID = array(":PDOUniqueId" => "$UpPointer[1]");

                $a = 1;

                while($a < $Lenght)
                {
                    $Values[$a] = Replace($Values[$a]);
                    $Param[$a] = Replace($Param[$a]);
                    $First = array("$Values[$a] = :$Values[$a]");
                    $Rez = array_merge($Rez, $First);
                    $First1 = array(":$Values[$a]" => "$Param[$a]");
                    $Set = array_merge($Set, $First1);

                    $a++;
                }

                $Final = implode(",", $Rez);
                $Bound = array_merge($Set, $MakeID);
                $Id = "$UpPointer[0]";

                try
                {
                    $stmt = $dbh->prepare("UPDATE $DB SET $Final WHERE $Id = :PDOUniqueId");
                    $stmt->execute($Bound);

                    $Affected = $stmt->rowCount();

                    if($Affected)
                    {
                        return true;
                    }
                    else
                    {
                        PDOError("Can't update DB");

                        return false;
                    }
                }
                catch (PDOException $e) 
                {
                    PDOError($e->getMessage());

                    return false;
                }
            }
            else
            {
                PDOError("Not all parameters");

                return false;
            }

        }
        else
        {
            PDOError("Not all parameters");

            return false;
        }

    }

    if($Type == "DELETE")
    {
        if(!empty($SeWhere) && !empty($SePointer))
        {
            try
            {
                $stmt = $dbh->prepare("DELETE FROM $DB WHERE $SeWhere = :user");
                $stmt->execute(array(":user" => $SePointer));

                $Affected = $stmt->rowCount();

                if($Affected)
                {
                    return true;
                }
                else
                {
                    PDOError("Can't delete from DB");

                }
            }
            catch (PDOException $e) 
            {
                PDOError($e->getMessage());

                return false;
            }

        }
        else
        {
            PDOError("Not all parameters");

            return false;
        }

    }

    PDOError("Unknown type of request");

    return false;
}

The function handles basic request such as update, insert, delete, select, count. In code it looks like this:

All values comes in array() form, which seemed like easiest solution:

UltimatePDO("UPDATE", "DB", "", "", array("Column"), array("Value"), array("hash", $user));

As for function Replace(), it just replaces unique symbols.

Answer 1

Poor cohesion

It's called logical cohesion when a routine that does multiple things of different logic. Your function will either insert records, or select records, or update records. These are very different operations, and their implementations should be in separate functions.

If you refactored this way, it would be much better:

if ($Type == "INSERT") performInsert(...)
elseif ($Type == "SELECT") performSelect(...)
elseif ($Type == "UPDATE") performUpdate(...)
// ...

In this form, by using an if-else chain to decide the next action and delegating to appropriate specialized functions, this function will have functional cohesion, which is the best kind of cohesion, characterized by a routine having a single purpose, in this case orchestrating database operations.

Mutually exclusive if

Notice that in the previous point I chained the conditions with elseif. In your code there are many if conditions that cannot happen together, so they should have been elseif.

Poor naming

I think, the common convention is to only use PascalCase for class names in PHP. Function names and variable names are usually either snake_case or camelCase. See also this related post.

---

It seems that $Param is expected to be an array, just like $Values. To indicate this fact better, I suggest making it plural, to $Params.

---

There are several problems with these names:

$Lenght = count($Values);
$Lenght1 = count($Param);

• The correct spelling is "length"
• Numbering in variable names is considered bad practice
• The "first" length being $Lenght, and the "second" length being $Lenght1 is just odd

I suggest renaming like this instead:

$valueCount = count($Values);
$paramCount = count($Param);

You should find better alternatives for the other numbered variables too, like $Rez1.

Answer 2

Usability

Your function will be extremely hard to use for anyone not familiar with the actual code (and in a month or two, that will probably include you as well). They only see this:

UltimatePDO($Type, $DB, $SeWhere = "", $SePointer = "", $Values = "", $Param = "", $UpPointer = "", $Extra = ""

There are quite a lot of variables, which makes it hard to remember their order and their meaning. The variable names also do not make it obvious what they represent, so there really is no chance of a user using it correctly without reading all the code.

You can try to help a user of your function with a PHPDoc style comment, but still, it will be hard to use.

To try to fix this, you should at least split the function into multiple functions. There's really not enough similarity between eg insert and select to put them both in the same function (this is why there are so many arguments, most of which would have to be set to null, because they are not needed for the desired query).

But generally, a wrapper like yours is not really recommended. PDO already has a neat interface which people are used to. Your function doesn't make it a lot easier to use, and people would first need to learn it. It also only provides a very limited subset of the PDO functionality. What if I want to insert multiple things? The query will be prepared each time, which is bad for performance. What if I want to join something? Or only select the columns I need?

If you still want a wrapper, you should look at the interface of existing code. A commonly used interface might look like this: getPDO()->select('column')->from('db')->where('column = :value')->setParam(':value', 'foo'). This is a lot easier to use than your code, because I don't have to remember the order of a bunch of arguments. An IDE will even auto-complete the function names.

Security

It is very unclear for a user which values can contain user input, and which cannot (these are actually most, eg $Extra, $DB, $SeWhere, $UpPointer, etc), which will cause security issues later on.

At the very least this should be well documented in PHPDoc comments.

Personally, I would probably not write a function which can easily be vulnerable if "wrongly" used. To solve this, you could add mandatory whitelist parameters for those values that cannot be prepared (and get rid of $Extra, which is way too broad and basically cannot be used securely).

global

global is often a code smell. It makes code harder to use, harder to test, etc. Just pass needed parameters as arguments.