patternphpMinor
Minimalistic mysql db class
Viewed 0 times
minimalisticmysqlclass
Problem
I am really just looking to make sure I am not making any weird mistakes that could hurt me in the long run or if something seems odd in the way I imagine it to work. This code does work for the way I use it and may lack other more advanced things that I may use in the future but have not implemented yet. I left in my phpdoc code to maybe make some sense of weird things. Any feedback is appreciated.
Note: I am constantly reading up on things and try to cover everything in each of the stack sites but I might have overlooked some very basic resources due to be newer to the stack family.
```
/**
* Class Main Mysql Database
*/
class mydb {
/**
* Connect to DB
*/
public function __construct() {
// Connect to Database
$this->mydb = new mysqli('host', 'database', 'user', 'pass');
if ($this->mydb->connect_errno) { // Error on connection failure
echo "Failed to connect to MySQL in Construct: (" . $this->mydb->connect_errno . ") " . $this->mydb->connect_error;
}
} /** End construct */
/**
* Choose items from the database (SELECT Statement with WHERE equals and like options)
* Designed to be slim and rely more on PHP than mysql for result formating
* @param string $select
* @param string $from
* @param string $config
* @param array $options
* @return array
*/
public function choose ($select, $from, $config = NULL, $options = NULL) {
if ($config === NULL) {
$stmt = "SELECT {$select} FROM {$from}";
} elseif ($config === 'EQUALS') {
$stmt = "SELECT {$select} FROM {$from} WHERE {$options['where_comp']} = '{$options['where_value']}'";
} elseif ($config === 'LIKE') {
$stmt = "SELECT {$select} FROM {$from} WHERE {$options['where_comp']} LIKE '{$options['where_value']}'";
} /** End if $config */
if (!($result = $this->mydb->query($stmt))) {
echo 'Query Error: ' . $result->error
Note: I am constantly reading up on things and try to cover everything in each of the stack sites but I might have overlooked some very basic resources due to be newer to the stack family.
```
/**
* Class Main Mysql Database
*/
class mydb {
/**
* Connect to DB
*/
public function __construct() {
// Connect to Database
$this->mydb = new mysqli('host', 'database', 'user', 'pass');
if ($this->mydb->connect_errno) { // Error on connection failure
echo "Failed to connect to MySQL in Construct: (" . $this->mydb->connect_errno . ") " . $this->mydb->connect_error;
}
} /** End construct */
/**
* Choose items from the database (SELECT Statement with WHERE equals and like options)
* Designed to be slim and rely more on PHP than mysql for result formating
* @param string $select
* @param string $from
* @param string $config
* @param array $options
* @return array
*/
public function choose ($select, $from, $config = NULL, $options = NULL) {
if ($config === NULL) {
$stmt = "SELECT {$select} FROM {$from}";
} elseif ($config === 'EQUALS') {
$stmt = "SELECT {$select} FROM {$from} WHERE {$options['where_comp']} = '{$options['where_value']}'";
} elseif ($config === 'LIKE') {
$stmt = "SELECT {$select} FROM {$from} WHERE {$options['where_comp']} LIKE '{$options['where_value']}'";
} /** End if $config */
if (!($result = $this->mydb->query($stmt))) {
echo 'Query Error: ' . $result->error
Solution
I basically subscribe to most of Pinoniq's remarks, but I'd like to touch on one thing in particular, that I just don't get.
It's good to see you're using
However, you are significantly reducing the features I can use: Where is the
No, so what would stop me from creating my own instance of
You attempt to sanitize the values you insert into a table, as you also sanitze the
What if I wanted to query the DB like so:
You're treating everything as strings, whereas I'm actualy using a boolean and integer value in the where clause. But that's not the real issue. The only way I can possibly get my query to execute via your class is to pass my fields and the
Now I know it's highly unlikely that user input will ever be passed through via these arguments, and it does at least allow users to perform a
With the post values being
Not what you want, I think...
The whole point of an abstraction layer should be to shield the users from having to write a ton of queries. Not all great devs are capable of writing great queries.
Basically, I'd advise you to not reinvent the wheel, but if you want to, as a technical exercise, look at the bigger picture: DB's consist of tables/collections. Each row of those tables holds a finite number of fields, and each field has a given type/dimension. The rows, therefore, can easily be poured into data objects. As can the tables, because, if you know the fields, you'll want to know how they fit in the table (key, primary, index, collation, storage engine...).
Take a look here, especially at the links below (ActiveRecord and ORM).
Read through a couple of pages, let it sit for a while, and think about how you'd go about implementing a simple query api.
And please, do use type-hints:
For example here:
If I passed a string there, or a numerically indexed array, and saw there was an error, and the message pointed to a line in your code, I'd tell you to debug your class. If a user doesn't pass the correct type of argument, he should be notified when he attempts to call the method. Change the signature to:
Even so, you're simply assuming certain keys will be set. Either use an object, that has the property you need, and defaults to a meaningful value, or compare the provided array to a private array that contains the default values:
Ah well, check a couple of my a
It's good to see you're using
mysqli_*, and I, for one, do understand why one would write a wrapper for this extension. It's API can be messy, and look confusing, and just allows for the users to sort-of switch from OO to procedural style, as they see fit. That's just wrong.However, you are significantly reducing the features I can use: Where is the
prepare method? How can I use transactions? How do I configure the connections? can't I link data models to rows of a given table?No, so what would stop me from creating my own instance of
mysqli and do what I want? There's no insentive for me to use your code, if anything, there are a few reasons why I wouldn't want to use your code. You attempt to sanitize the values you insert into a table, as you also sanitze the
where clauses. You're not using prepared statements, though, why?What if I wanted to query the DB like so:
SELECT clients.first_name, clients.last_name, contacts.email FROM db.clients
LEFT JOIN db.contacts
ON contacts.client_id = clients.id
WHERE clients.language = 1
AND clients.active = TRUE;You're treating everything as strings, whereas I'm actualy using a boolean and integer value in the where clause. But that's not the real issue. The only way I can possibly get my query to execute via your class is to pass my fields and the
JOIN as the $from parameter, since it's just being concatenated in there, without any check on it whatsoever.Now I know it's highly unlikely that user input will ever be passed through via these arguments, and it does at least allow users to perform a
JOIN. Still, you have to agree this is a rather hacky workaround for a problem that needn't exist in the first place, and it does mean that, in theory, this is possible:$mydb->choose($_POST['input'], $_POST['input2']);With the post values being
FROM users / and */ respectively, which means, you're executing this query:SELECT * FROM users /* FROM */Not what you want, I think...
The whole point of an abstraction layer should be to shield the users from having to write a ton of queries. Not all great devs are capable of writing great queries.
Basically, I'd advise you to not reinvent the wheel, but if you want to, as a technical exercise, look at the bigger picture: DB's consist of tables/collections. Each row of those tables holds a finite number of fields, and each field has a given type/dimension. The rows, therefore, can easily be poured into data objects. As can the tables, because, if you know the fields, you'll want to know how they fit in the table (key, primary, index, collation, storage engine...).
Take a look here, especially at the links below (ActiveRecord and ORM).
Read through a couple of pages, let it sit for a while, and think about how you'd go about implementing a simple query api.
And please, do use type-hints:
For example here:
public function choose ($select, $from, $config = NULL, $options = NULL)
{
//some stuff
$options['some_key'];
}If I passed a string there, or a numerically indexed array, and saw there was an error, and the message pointed to a line in your code, I'd tell you to debug your class. If a user doesn't pass the correct type of argument, he should be notified when he attempts to call the method. Change the signature to:
public function choose ($select, $from, array $config = NULL, array $options = NULL)Even so, you're simply assuming certain keys will be set. Either use an object, that has the property you need, and defaults to a meaningful value, or compare the provided array to a private array that contains the default values:
class FoolMe
{//minimal checks
private $defaults = array(
'name' => array('type' => 'string', 'value' => 'default'),
'age' => array('type' => 'integer', 'value' => 0)
);
public function checkArray(array $arr)
{
foreach($this->defaults as $k => $settings)
{
if (isset($arr[$k]))
{
$arr[$k] = gettype($arr[$k]) === $settings['type'] ? $arr[$k] : $settings['value'];
}
else
{
$arr[$k] = $settings['value'];
}
}
return $arr;
}
}
class SelectOptions
{
protected $where_comp = null;
public function __get($name)
{//bad, but better than array
if (property_exists($this, $name))
{
return $this->{$name};
}
}
//best:
public function getWhereComp()
{
if ($this->where_comp === null)
{
return '';
}
return (string) $this->where_comp;
}
public function setWhereComp($str)
{
$this->where_comp = (string) $str;//ensure types using setters
}
}Ah well, check a couple of my a
Code Snippets
SELECT clients.first_name, clients.last_name, contacts.email FROM db.clients
LEFT JOIN db.contacts
ON contacts.client_id = clients.id
WHERE clients.language = 1
AND clients.active = TRUE;$mydb->choose($_POST['input'], $_POST['input2']);SELECT * FROM users /* FROM */public function choose ($select, $from, $config = NULL, $options = NULL)
{
//some stuff
$options['some_key'];
}public function choose ($select, $from, array $config = NULL, array $options = NULL)Context
StackExchange Code Review Q#29670, answer score: 5
Revisions (0)
No revisions yet.