Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

this is my first post on the whole StackExchange network, so I might make some mistakes. And I'm also spanish spearker, so I'll probably have mistakes in my writting too..

Context: I'm starting with object oriented programming, but I know php prodecudral style "pretty well" i'd say. So, I'm working on a ticket support site for this website. This is how it goes:

This mysqliSingleton Class was in a question I previusly visitted. I only added the set_charset() methedo to feet my database. By the way, should that line be in the __constructor or in the init method?

class mysqliSingleton {
private static $instance;
private $connection;

private function __construct() {
    $this->connection = new mysqli(DB_SERVER, DB_USERNAME, DB_PASSWORD, DB_DATABASE);
    $this->connection->set_charset('utf8');
}

public static function init() {
    if(is_null(self::$instance)) {
        self::$instance = new mysqliSingleton();
    }
    return self::$instance;
}


public function __call($name, $args) {
    if(method_exists($this->connection, $name)) {
         return call_user_func_array(array($this->connection, $name), $args);
    } else {
         trigger_error('Unknown Method ' . $name . '()', E_USER_WARNING);
         return false;
    }
}
}

This is a piece of my ticket class wich also has other methos delete, getFromUser, getAll, and some other:

class Ticket extends mysqliSingleton {
private $mysqli;
public function __construct() {
    $this->mysqli = mysqliSingleton::init();//Singleton db
}
//Submit a ticket
public function submitTicket($idusuario, $problem,$moreinfo){
    $query = "INSERT INTO tbl_name (tbl_field,tbl_problem,tbl_moreinfo) VALUES (?,?,?)";
    $stmt = $this->mysqli->prepare($query) or die($this->mysqli->error);
    if ($stmt) {
        $stmt->bind_param('iss',$idusuario,$problem,htmlentities($moreinfo));
        if ($stmt->execute()) {
            return true;
        } else return false;//Problem sql
    }
}
}

So my main question would be to know if I'm using singleton pattern right.

But (unfortunatly) after I code that, I starting reading more about abstract classes and Interfaces, and I've started to doubt if this is right. By the way, the ticket class, is one of many classes that will extend to mysqliSingleton. I'm sorry if it's too long but im kinda new to this question&andswer stuff :P

Thank you very much.

share|improve this question

migrated from stackoverflow.com Dec 13 '13 at 21:43

This question came from our site for professional and enthusiast programmers.

1  
Question: why are you applying htmlentities on a string before you store it in a database? That function (or rather: htmlspecialchars) is for outputting text in an HTML context. –  Marcel Korpel Dec 13 '13 at 16:07
    
Don't you get bored writing mysqliSingleton every time you need a database call? –  Your Common Sense Dec 13 '13 at 16:10
    
Becouse I expect the user will not to insert any HTML attributes. Well, i'm deffining a variable $mysqli so I do $this->mysqli->whatEverMethod(). How should I do that? –  LCH Dec 13 '13 at 16:11
1  
Implementing Singletons in PHP make no sense. If you need the same instance in multiple clases use Dependency Injection. In PHP Objects do not live in the application memory, a singleton created for one request lives exactly for that request only. That beats the purpose of a singleton. –  AlanChavez Dec 13 '13 at 16:13
3  
Please don't use 'singleton' and 'OOP' in same sentence. –  tereško Dec 13 '13 at 17:25

2 Answers 2

up vote 2 down vote accepted

Well, although this code belongs to Codereview@stackexchenge, but as nobody cares to follow any rules here - so I would.

There are too much problems with your code, ut to outline most:

  1. It seems you don't quite understand what singleton is and why use it.
  2. There is no point in extending application class from database handler class.
  3. trigger_error('Unknown Method ') is apparently redundant. PHP can handle absent methods as well.
  4. Never use die() in production code.
  5. This whole class makes very little sense as it don't help you to handle mysqli even a bit.

Frankly, exactly the same result you can have without any "singleton" class but just by creating mysqli instance and making it

public function __construct() {
    global $mysqli;
    $this->mysqli = $mysqli;
}
share|improve this answer
    
You have access to moderator tools, can't you just move it to codereview? –  AlanChavez Dec 13 '13 at 16:22
    
1. You are right, as I've said im new to OOP stuff. 2. You're completly right, I haven't realized that!! 3. removed right now. 4. im not in producition mode. but thanks. 5. i didn't really understood this point! –  LCH Dec 13 '13 at 16:24
1  
@AlanChavez I will start using moderator tools not sooner than at least 50% of local population will start follow site rules (namely: closing duplicated and offtopic questions). Means never. –  Your Common Sense Dec 13 '13 at 16:27
    
@LCH 1. Don't use it then. 2. Just added a perfect link. 4. Any code is production. Are you going to go through ALL the code and change dirty stuff like that then moving to production? Why not to make it ALREADY the right way? 5. See example of class then helps: github.com/colshrapnel/safemysql –  Your Common Sense Dec 13 '13 at 16:30
    
Your Common Sens, i'm sorry but I didnt' really understoo your edit: if I do that, wouldn't i be creating a new mysqli instance everytime i create a new object? –  LCH Dec 13 '13 at 16:30
public static function init() {
    if(is_null(self::$instance)) {
        self::$instance = new self();
    }
    return self::$instance;
}
share|improve this answer
    
Ok that's an improvment but it basically does the same thing, or does PHP handle different the self() method than doing a new whatEverClass() ? –  LCH Dec 13 '13 at 16:09

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.