Honestly, you can improve your application by discarding this wrapper. You are not going to derive any value from it.
You can find similar discussion around this very common Codereview topic in questions such as those I have listed below.
Class for reducing development time
Simple PDO database class in PHP
Phishing Project Error Logging
What you should hopefully take away from this is that you are adding an unnecessary level of abstraction around an existing database abstraction (PDO) and adding no additional value to it. Typically one might write database-related classes to do things like:
- connection management (like a Singleton or connection pool implementation)
- adding "model" capabilities (i.e. ORM, active record pattern, etc.)
- adding fluent, natural language query capabilities (
$db->select('field')->from('table')->where('id = ?') or similar).
You are doing none of that. You are just obfuscating the underlying PDO implementation from the caller, while simultaneously leaking implementation details outside the class (you are throwing
PDOException for example).
Some specific comments on code
- Consider moving your database configuration to outside of your code. There is no reason login credentials should be present in any of your code. Ideally you derive these from environmental configuration.
- Why are you defining variables in your parameters here =>
public function __construct($host = DB_HOST , $data = DB_DATA , $user = DB_USER , $pass = DB_PASS)? Why do you even need parameters if these values are defined as constants? You could build your DB connection string in your constructor directly from the constants. Are you really expecting to have to make dynamic/runtime assignment for these host and login credentials as you are trying to allow for?
- Don't echo out error messages to standard out (i.e.
die('message')). These errors should be logged and an appropriate exception raised to inform the caller of the error. It should not be the place of this class to determine end-user messaging. The calling code is better positioned to do this.
- You should validate parametric data passed into public methods. Here on your
query() method you do nothing to valdiate you have a reasonable query string to work with. You should validate and throw appropriate
InvalidArgumentException or similar if validation fails. You don't to try to make queries against the database when an empty string is passed, for example.