Please review my classes and give me some advice to improve them. I have also put some additional questions in the code and I hope you can give me an answer for some of them.
Database.php
<?php
class Database
{
private static $dsn = 'mysql:host=localhost;dbname=knight;charset=UTF-8';
private static $user = 'root';
private static $pass = '';
private static $instance = null;
private function __construct()
{}
//is PDO+Singleton a good combination?
public static function getInstance()
{
if(!isset(self::$instance))
{
try
{
self::$instance = new PDO(self::$dsn, self::$user, self::$pass);
self::$instance->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
catch (PDOException $e)
{
echo 'Error found: '.$e->getMessage();
}
}
return self::$instance;
}
//Should i make methots prepare(), execute(), bindValue() etc. and why?
}
?>
Validator.php
<?php
class Validator
{
//All the methods are static, is this a good practice for a class like this?
public static function isValidMail($var)
{
return preg_match("/([\w\-]{3,40}+\@[\w\-]{3,120}+\.[\w\-]{2,6}+)/", $var);
}
public static function isLong($var, $max)
{
return(mb_strlen($var, 'UTF-8') > $max);
}
public static function isShort($var, $min)
{
return (mb_strlen($var, 'UTF-8') < $min);
}
public static function isValidID($var)
{
$var = (int)$var;
return ($var > 0);
}
}
User.php
<?php
class User
{
//Should I have properties such as username, password, email etc. and why?
private $db = null;
private $errors = array();
public function __construct($db)
{
$this->db = $db;
}
public function register($username, $pass, $mail)
{
if($this->validate($username, $pass, $mail))
{
$username = $this->filter($username);
$mail = $this->filter($mail);
$exists = $this->exists($username, $mail);
if(!$exists)
{
$pass = $this->hash($pass);
$time = time();
//Should I use exceptions for the SQL queries in this class?
$rs = $this->db->prepare("INSERT INTO users(username, mail, pass, date_reg) VALUES(:name, :mail, :pass, :time)");
$rs->execute(array(':name' => $username, ':mail' => $mail, ':pass' => $pass, ':time' => $time));
header('Location: index.php');
die();
}
else
{
$this->errors[] = 'Username or email already exists!';
}
}
if(count($this->errors > 0))
{
$this->showErrors();
}
}
public function login($username, $pass)
{
if($this->validate($username, $pass))
{
$username = $this->filter($username);
$exists = $this->exists2($username, $pass);
if($exists)
{
$_SESSION['logged'] = 1;
$_SESSION['data'] = $exists->fetch(PDO::FETCH_ASSOC);
header('Location: index.php');
die();
}
else
{
$this->errors[] = 'Wrong username or password!';
}
}
if(count($this->errors > 0))
{
$this->showErrors();
}
}
public function exists($username, $mail)
{
$exists = $this->db->prepare("SELECT user_id FROM users WHERE username = :name OR mail = :mail");
$exists->execute(array(':name' => $username, ':mail' => $mail));
return ($exists->rowCount() > 0);
}
public function exists2($username, $pass)
{
$pass = $this->hash($pass);
$exists = $this->db->prepare("SELECT * FROM users WHERE username = :name AND pass = :pass");
$exists->execute(array(':name' => $username, ':pass' => $pass));
if($exists->rowCount() > 0) return $exists;
return false;
}
public function validate($username, $pass, $mail = null)
{
if(Validator::isShort($username, 4)) $this->errors[] = 'Short username!';
elseif(Validator::isLong($username, 25)) $this->errors[] = 'Long username!!';
if(Validator::isShort($pass, 5)) $this->errors[] = 'Short password!';
if($mail !== null)
{
if(!Validator::isValidMail($mail))
{
$this->errors[] = 'Invalid email!';
}
}
return (count($this->errors) == 0);
}
public function getUserData($id)
{
if(Validator::isValidID($id))
{
$rs = $this->db->prepare("SELECT * FROM users WHERE user_id = ?");
$rs->execute(array($id));
if($rs->rowCount() == 1) return $rs->fetch(PDO::FETCH_ASSOC);
return false;
}
return false;
}
private function hash($var)
{
return hash('sha512', $var);
}
public function filter($var)
{
//Is htmlentities enough if I use PDO?
return htmlentities($var, ENT_QUOTES, 'UTF-8');
}
private function showErrors()
{
foreach($this->errors as $e)
{
//I am outputting text in the class, is that bad?
echo '<strong>'.$e.'</strong><br />';
}
}
}