Database wrapper class

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

ole wrote:
I believe... C++ makes copies, but I think PHP creates references
PHP 4 and C++ both copy and are both ridiculous for it. Most languages reference including PHP 5.
Well, it's good and bad. Good because you don't risk harming/changing the original data, but bad because it's less efficient. I'm fine with it in C++ because you can easily turn any variable into a reference, though ampersands everywhere tends to look messy. Maybe they should have put it the other way around.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

DIRECTORY_SEPARATOR is the proper thing to have. Stick with it.
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

Code: Select all

str_replace('/', DIRECTORY_SEPARATOR, 'foo/bar/whatever.php');
Maybe? Hmm? Hmm? :-p
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

ole wrote:Also have you considered following Zend or PEAR coding standards? See the first link in my sig.
Negative. I probably should, but I'm content with my own coding style. When I get good at this stuff, and writing something other people may want to contribute on or work with, then I'll learn the coding standards.

Although, I've read the standards before. I could do them but I just choose not to. For now I like to stick with underscores and all lowercase letters.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

A couple of comments.

1. As for isError() and getError() they would return true/false or a string respectively. They are used to check if an error occurred by saving the error state after the query or fetch.

2. I prefer to always return an object from the query() method. It makes it consistent to deal with rather than it being either an object or false. Especially if you pass it, then the dependency is clean/clear.

3. I would recommend that you change the naming now. It is not a good practice become too familiar with you own non-standard naming. I consider being open and flexible to the advance of standards to be a sign of a mature programmer.
(#10850)
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

IMO, the standard doSomethingHere is much uglier and harder to deal with thatn do_something_here, but eh, standards are standards, and who am I?
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

scottayy wrote:IMO, the standard doSomethingHere is much uglier and harder to deal with thatn do_something_here, but eh, standards are standards, and who am I?
I think the do_something_here is the style they use for you to be able to differentiate between built-in functions and your own. (Correct me if I'm wrong.)

But yeah, I don't like have the first letter of a function lowercased... The standards I code by are most derived from C++, however typing the dollar sign made me stop using prefixes... made me press shift too often. :-p
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

So, how's this conform to the standards (besides documentation blocks)

Code: Select all

<?php

class db_connect
{
    private $_host;
    private $_username;
    private $_password;
    private $_linkId;
    
    public function __construct($host, $username, $password)
    {
        $this->_host = $host;
        $this->_username = $username;
        $this->_password = $password;
    }
    
    public function connect()
    {
        $this->_linkId = mysql_connect($this->_host, $this->_username, $this->_password);
    }
    
    public function useDb($db)
    {
        mysql_select_db($db, $this->_linkId);
    }
    
    public function escape($var)
    {
        return mysql_real_escape_string($var, $this->_linkId);
    }
    
    public function query($sql)
    {
        if (!$this->_linkId) {
            $this->connect();
        }
        
        if (is_resource($result = mysql_query($sql, $this->_linkId))) {
            require_once getcwd() . DIRECTORY_SEPARATOR . 'class' . DIRECTORY_SEPARATOR . 'db_result.class.php';
            return new db_result($result);
        }
        
        return false;
    }
}

Code: Select all

<?php

class db_result
{
    private $_result;
    
    public function __construct($result)
    {
        $this->_result = $result;
    }
    
    public function numRows()
    {
        return mysql_num_rows($this->_result);
    }
    
    public function fetchRow($fetch_type=MYSQL_ASSOC)
    {
        return mysql_fetch_array($this->_result, $fetch_type);
    }
}
User avatar
vigge89
Forum Regular
Posts: 875
Joined: Wed Jul 30, 2003 3:29 am
Location: Sweden

Post by vigge89 »

You asked for articles on some of the stuff in OO earlier, and since I'm in a rush I'll just post them if you're still interested:
http://hudzilla.org/phpwiki/index.php?t ... rogramming
Something I read yesterday (just started with OOP), I found the article quite good.
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Post by s.dot »

Thank you for that link! It seems very in depth. I read a few pages and added it to my favorites.
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

I don't know if everyone will agree, but the query method

Code: Select all

public function query($sql)
    {
        if (!$this->_linkId) {
            $this->connect();
        }
       
        if (is_resource($result = mysql_query($sql, $this->_linkId))) {
            require_once getcwd() . DIRECTORY_SEPARATOR . 'class' . DIRECTORY_SEPARATOR . 'db_result.class.php';
            return new db_result($result);
        }
       
        return false;
    }
Should have no business requiring the file itself. Either that should be re factored into it's own method of the result class is simply lazy loaded. Probably the latter.

The next step would be supporting result operations for non-select's. For instance, currently numRows() and fetchRow() have no business for in update/insert operation. Polymorphism could be applied to retrieve the correct result class depending on the query itself.
thinsoldier
Forum Contributor
Posts: 367
Joined: Fri Jul 20, 2007 11:29 am
Contact:

Post by thinsoldier »

why does one class handle both connecting and querying and another class for the results?

Code: Select all

require_once getcwd().DIRECTORY_SEPARATOR.'class'.DIRECTORY_SEPARATOR.'db_result.class.php';
                        return new db_result($this->link_id, $result); 

see: http://bs.php.net/autoload

By the second page I definitely don't understand the need for the result to by handled by a separate class. Anyone able to explain why to me?[/syntax]
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

They're separate things. They have separate verbs and data.
thinsoldier
Forum Contributor
Posts: 367
Joined: Fri Jul 20, 2007 11:29 am
Contact:

Post by thinsoldier »

feyd wrote:They're separate things. They have separate verbs and data.
could i get a better explanation?
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

thinsoldier wrote:
feyd wrote:They're separate things. They have separate verbs and data.
could i get a better explanation?
They have separate responsibilities, therefore should be represented as different objects. Yes, you certainly could put them into a single object, but thats not good OOP.
Post Reply