SitePoint Sponsor

User Tag List

Results 1 to 18 of 18

Thread: Code critique?

  1. #1
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    Connecticut
    Posts
    173
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Code critique?

    I think I've become a pretty decent PHP programmer--I use OO, SQL, and all that jazz.

    However, is there any way to get my code critiqued? I don't think it would make sense to just post a few dozen sources here, so I'm really not sure how to go about it, or if anyone would even be willing to take a look.

  2. #2
    Programming Team silver trophybronze trophy
    Mittineague's Avatar
    Join Date
    Jul 2005
    Location
    West Springfield, Massachusetts
    Posts
    16,441
    Mentioned
    160 Post(s)
    Tagged
    1 Thread(s)

    code critique

    I think if you keep the code examples to less than head splitting size and phrase the questions clearly you'd probably get at least a few comments. I have seen quite a few threads that are like
    I want to do this ....
    I am doing it it this way ....
    Is this the best way?
    I don't think many people want to digest the code for an entire app, but if you can generalize the big picture and detail the snippets, give it a try.

  3. #3
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    Connecticut
    Posts
    173
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    You're probably right, I don't think I'd want to run through five hundred lines of code myself

    My biggest concern right now is my SQL class.. it does the job but somehow I think it could be "cleaner"--
    PHP Code:
    class SQL
    {
        function 
    SQL()
        {
            global 
    $config;
            
    $this->connect mysql_connect($config['host'],$config['uname'],$config['upass'])
                or die(
    mysql_errno().': '.mysql_error());
            
    $this->db mysql_select_db($config['db'],$this->connect)
                or die(
    mysql_errno().': '.mysql_error());
        }
        
    # Query
        
    function Query($query,$file 'none',$line 'none')
        {
            global 
    $safe;
            return @
    mysql_query($query) or $this->Error($file,$line,mysql_errno(),mysql_error());
        }
        
    # Data
        
    function Fetch($fetch,$file 'none',$line 'none')
        {
            return @
    mysql_fetch_assoc($fetch) or $this->Error($file,$line,mysql_errno(),mysql_error());
        }
        
    # Data directly from query
        
    function Result($result,$file 'none',$line 'none',$noError false)
        {
            return 
    $this->Fetch($this->Query($result,$file,$line),$file,$line) or $this->Error($file,$line,mysql_errno(),mysql_error(),$noError);
        }
        
    # Number of rows
        
    function Rows($rows,$file 'none',$line 'none')
        {
            return @
    mysql_num_rows($rows) or $this->Error($file,$line,mysql_errno(),mysql_error());
        }
        
    # Error
        
    function Error($file,$line,$errno,$error,$noError false)
        {
            if(
    $noError != true) {
                die(
    "<br>File: $file<br>Line: $line<br>Error Number: ".mysql_errno().'<br>Error: '.mysql_error());
            }
        }

    There probably isn't a better way to generate "fancy" errors though.

  4. #4
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    You should probably post OO code on the advanced forum.

    PHP Code:
    class SQL 
    The name is not good. What does a SQL class actually do? SQL is a data programming language. Is your class a data programming class? I think your class is more to do with sending queries to the database. It's a single point of connection if you like. Better to call it MysqlConnection or MysqlQueryer. Words ending in"er" are usually good first tries. A clear well designed class has a very obvious role.

    PHP Code:
        function SQL()
        {
            global 
    $config
    Don't use globals for anything ever. You should have passed this variable in. Here is a small selection of the problems you could run into...

    1) Say some other object changed the configuration causing a failure - which one? You have narrowed down the bug to your entire code base. You have lost encapsulation in other words. You have lost the ability to divide and conquer.
    2) If you change the $configuration for this class into an object say, it will affect all the other code. There is no easy way to transition to another configuration system, making it hard to change. Again you have lost encapsulation in taht you cannot change one part of teh system without changing others. Shotgun surgery as it's called. It's a bad sign.
    3) Another library (or an old version of your own) decides to use that variable as well.
    4) How do you know that the global has been set up before you use it. If you passed in a variable, then the contract is obvious.
    5) How do other developers know that your class uses the configuration? They will get very confused when they try to use it. They will have to read your code line by line to figure out what is happening. That defeats the whole purpose of objects, hiding internal detail.

    PHP Code:
            $this->connect mysql_connect($config['host'],$config['uname'],$config['upass'])
                or die(
    mysql_errno().': '.mysql_error()); 
    Objects at this low a level should never die. Stopping the script is a high level decision, and that decision should be made in the top level script. A user of your class in going to be rather surprised when the whole page dies. What if they were half way through something important and need to recover?

    You should set an error in the object in PHP4, or better throw an exceptionin PHP5.

    PHP Code:
        # Query 
    The comment is just the same name as the method. It's useless.
    PHP Code:
        function Query($query,$file 'none',$line 'none'
    Why are you telling your class what line number it's caled from. This is nothing to do with database stuff at all (each class should do one thing well). Dump it and install XDebug or use the stack_trace() functions in your error handler. You want to remove any clutter.
    PHP Code:
            global $safe
    This one isn't even used.
    PHP Code:
            return @mysql_query($query) or $this->Error($file,$line,mysql_errno(),mysql_error()); 
    Do you really want to kill the program on every failed query? You are returning a raw query result, which kind of defeats the purpose of hiding the details in a class. In fact this class just seem sto be a wrapper for the mysql_* functions. More later.
    PHP Code:
        # Data 
    This comment is even more useless than the last one.
    PHP Code:
        function Fetch($fetch,$file 'none',$line 'none'
    You have to pass that handle back into the class. That's confusing.Why can't your class handle all of this, or create another object. See later.
    PHP Code:
        # Data directly from query
        
    function Result($result,$file 'none',$line 'none',$noError false)
        {
            return 
    $this->Fetch($this->Query($result,$file,$line),$file,$line) or $this->Error($file,$line,mysql_errno(),mysql_error(),$noError);
        } 
    Both the comment and the method name are totally misleading. Why not call it selectSingleRow($sql)?
    PHP Code:
        # Number of rows 
    If you just rename the method to this then you don't need the comment.
    PHP Code:
        function Rows($rows,$file 'none',$line 'none')
        {
            return @
    mysql_num_rows($rows) or $this->Error($file,$line,mysql_errno(),mysql_error());
        } 
    This is also buggy as you can get row counts from a different query from the one you are currently fetching if there has been a query in the meantime. If you want this information then capture it at the time you do the query.
    PHP Code:
        # Error 
    Why?
    PHP Code:
        function Error($file,$line,$errno,$error,$noError false)
        {
            if(
    $noError != true) {
                die(
    "<br>File: $file<br>Line: $line<br>Error Number: ".mysql_errno().'<br>Error: '.mysql_error());
            }
        } 
    This does nothing beyond die. At least if you stashed the error, and then had a getError() method to read it back, the top level script could choose if it wanted to die or not.

    Also how do you close the connection once opened?

    On the plus side, the methods were short and easy to understand. That counts for a lot.

    Here is an alternative solution. We'll have two objects, one for sending queries and one for looping through results...
    PHP Code:
    class MysqlConnection {
        function 
    MysqlConnection($username$password$db$host 'loalhost') { ... }
        function 
    select($sql) { ... }
        function 
    selectSingleRow($sql) { ... }
        function 
    execute($sql) { ... }
        function 
    close() { ... }
        function 
    isError() { ... }
        function 
    getError() { ... }
    }

    class 
    MysqlResult {
        function 
    MysqlResult($result_handle$count$error false) { ... }
        function 
    next() { ... }
        function 
    getCount() { ... }
        function 
    isError() { ... }
        function 
    getError() { ... }

    The execute method is for queries such as deletion that don't return any results.

    Usage is something like this...
    PHP Code:
    $connection = new MysqlConnection(...);
    if (
    $connection->isError()) {
        die(
    $connection->getError());
    }
    $result $connection->select('select * from people');
    if (! 
    $result->isError()) {
        while (
    $row $result->next()) {
            
    // Show the row.
        
    }

    It's the usage example (or test case) that really determines the design of these low level classes. Sketch one out before coding as it's a great help.

    The only tricky method here is the select()...
    PHP Code:
    class MysqlConnection {
        ...
        function 
    select($sql) {
            
    $handle mysql_query($sql$this->_connection_id);
            if (! 
    $handle) {
                
    $error mysql_get_error($this->_connection_id);
            }
            return new 
    MysqlResult(
                    
    $handle,
                    
    mysql_num_rows($this->_connection_id),
                    
    $error);
        }

    I am sure you can fill the rest of the code in yourself. The MysqlResult does the associative array fetch and stashes the row count. this means you can run another query whilst still reading the results from the first. the result sets are independent.

    You see this set-up a lot in many guises. Often transactions are added to the connection,

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  5. #5
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    Connecticut
    Posts
    173
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Thanks Marcus, I appreciate the runthrough

    Before I rewrite the code, there's a few issues I'd like to get out of the way, so I don't end up forcing myself to rewrite some of the above problems.

    First, is global $class; appropriate for making other classes available in the current class? I have a class that cleans superglobals, but I need to global it to make it available, and it has the inherent problem of requiring that the class Safe has been instantiated.

    Second, what's a better way to access config data? I was thinking of creating a class to preserve static copies of the variables during the script execution, so I could call them without an issue. But again, it has the problem outlined above.

    Last, what would be the best way to go about creating a generic class to access the mysql-specific functions? IE, I would do $dao->execute(...); and it would decide whether to import a mysql class or a pgsql class based on the configuration.

  6. #6
    SitePoint Wizard dreamscape's Avatar
    Join Date
    Aug 2005
    Posts
    1,080
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    You should have passed this variable in. Here is a small selection of the problems you could run into...

    2) If you change the $configuration for this class into an object say, it will affect all the other code. There is no easy way to transition to another configuration system, making it hard to change. Again you have lost encapsulation in taht you cannot change one part of teh system without changing others. Shotgun surgery as it's called. It's a bad sign.
    You get that same problem passing it in as variable too. If you pass it in as an array variable, and then later decide to change it to an object, it still affects all code that expected it to be an array! Even with encapsulation, you can't change something to be something other than what the other parts of the system expect it to be like without also changing the other parts.

  7. #7
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Quote Originally Posted by dreamscape
    You get that same problem passing it in as variable too. If you pass it in as an array variable, and then later decide to change it to an object, it still affects all code that expected it to be an array! Even with encapsulation, you can't change something to be something other than what the other parts of the system expect it to be like without also changing the other parts.
    OK, I should have explained taht one better.

    Passing it in I can pick and choose without touching the innards of the class. This means I can make the changeover class by class, rather than in one go. This is the difference between the change being realistic and it never happening.

    Suppose we change the MysqlConnection first...
    PHP Code:
    $connection = new MysqlConnection(new ConfigurationAsObject($configuration)); 
    The main value of this is in using different versions of different libraries. The ConfigurationAsObject is an adapter pattern.

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  8. #8
    SitePoint Wizard dreamscape's Avatar
    Join Date
    Aug 2005
    Posts
    1,080
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I understand that marcus, but you were speaking directly about the global $config variable, which in this guys code is an array. And I was merely stating that even if one passes it in as a variable (it would still be an array), and then later decides to change the config from an array to an object, you still have to go and change it every place you use it.

    If you code your classes expecting your program config to be an array, it has to be an array form, period. You cannot change it to an object without changing the code in your classes. Likewise, if you code your classes expediting the config to be an object, you cannot have it be something else without changing the code in your classes, period. Unless you jimmy-rig it to turn the array into an object or the object into an array because you might be too lazy to change your class code, or probably better, temporarily maintain both a config array and config object while you depreciate one or the other with the goal of ultimately remove one from the codebase.

    I am going through this exact thing right now, having moved the program config from a global array to an object held in a commonly accessible object registry. Until I can change all of the code that thinks it is an array to use it as an object, I've still got to have the global array available, but have it flagged as depreciated, and will ultimately drop it completely from the code. (Yeah true it would have been easier to make the config as an object from the start instead of a global array, but hindsight is always 20/20).

    But the point is, you cannot make structural change like that to a critical part of the system (I think the program config qualifies as highly critical) and not have it affect other parts of the program, no matter how "separated" all the parts are.

  9. #9
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Quote Originally Posted by dotDan
    First, is global $class; appropriate for making other classes available in the current class? I have a class that cleans superglobals, but I need to global it to make it available, and it has the inherent problem of requiring that the class Safe has been instantiated.
    This is typical of the problems you get into with globals. Why does the superglobal stuff have to be used once you have OO? In the top level script do this...
    PHP Code:
    $request = new SafeRequest($_REQUEST); 
    Then pass the request object around. There is a big advantage here in that you can set up your own versions for test scripts (very important).

    Quote Originally Posted by dotDan
    Second, what's a better way to access config data? I was thinking of creating a class to preserve static copies of the variables during the script execution, so I could call them without an issue. But again, it has the problem outlined above.
    It really depends on where the configuration is stored. If it's hard coded stuff, then use a class...
    PHP Code:
    class Configuration {
        function 
    getDatabseUsername() { ... }
        function 
    getPaymentGatewayUrl() { ... }
        ...

    In fact it's worth starting an application this way until the business basics have been fleshed out. Configuration is technical stuff that you sort out when your client isn't available.

    Once you have it in it's own class you can extend it anyway you want: load a file behind the scenes, code generate some PHP from XML, environment variables or whatever.

    That you can just pass this object around often surprises people. After all, configuration is used by everything right? Well no. The number of objects that actually need it is very small, but they are often deeply buried and hard to reach. There are two options...

    1) Have higher level concepts in higher layers of the application. Then pass it into that object. So you have a DB of people? Have a PersonRepository that takes in the configuration. IT then passes just the needed parameters down to DAOs, DB connections or whatever. This is the factory system.

    2) Use the passing as a sign that some refactoring is needed and there are too many layers. It may be that you simply need to create the objects that need the configuration higher up, MysqlConnection say, and then pass the object down instead. This is usually clearer anyway. If you see a Configuration object being passed, you have no idea what is going to happen. If you see a MysqlConnection being passed you have a very clear idea of kind of damage can be done by that object.

    Try it. You'll be pleasantly surprised, I promise.

    Quote Originally Posted by dotDan
    Last, what would be the best way to go about creating a generic class to access the mysql-specific functions? IE, I would do $dao->execute(...); and it would decide whether to import a mysql class or a pgsql class based on the configuration.
    This starts to get us into the heavy stuff.

    Just write another class with the same interface...
    PHP Code:
    class PostgresConnection {
        function 
    PostgresConnection($username$password$db$host 'loalhost') { ... }
        function 
    select($sql) { ... }
        function 
    selectSingleRow($sql) { ... }
        function 
    execute($sql) { ... }
        function 
    close() { ... }
        function 
    isError() { ... }
        function 
    getError() { ... }

    Now you simply pass ito your code the PostgresConnection instead of the MysqlConnection. It will be none the wiser. Remember when I said passing the connection was better than passing the configuration? Well, this is an example of the benefit.

    YOu can go a step further here. Have a method on a class somewhere (DAO, repository) which knows which connection to create. Your other code can go...
    PHP Code:
    $factory = new Database($configuration);
    $connection $factory->getConnection(); 
    Again, you see this quite a lot. It's called the factory pattern.

    There is a complication here, and this may overload you a bit. It certainly overloaded me when I came across it . You can use the same trick again and again.

    You will probably want different SQL syntax for different DBs. One way out is to have a class build the queries...
    PHP Code:
    class SqlBuilder {
        function 
    select($table$where_clauses) { ... }
        ...

    This is a simple one. Tailor it to the queries that you need. It just outputs the strings it needs.

    Next we split it...
    PHP Code:
    class MysqlSqlBuilder extends SqlBuilder { ... }
    class 
    PostgresSqlBuilder extends SqlBuilder { ... } 
    Now we have a problem. If we create a MysqlConnection, we want a MysqlSqlBuilder to go with it. We will have some very tricky bugs if we mix things up.

    Back to our factory...
    PHP Code:
    class Database {
        function 
    createConnection() { ... }
        function 
    createSqlBuilder() { ... }
        function 
    selectDatabse($configuration) { ... }
    }

    class 
    Mysql extends Database { ... }
    class 
    Postgres extends Database { ... } 
    Our Database factory has become abstract, and the pattern is called the "abstract factory". It creates families of objects, here Connection and Sql, and if you want to swap the family, you just swap the factory. If you have ever wondered how large systems can change their entire database infrastructure just by changing a configuration entry, that's how.

    How do we choose which DB to use? By a factory method of course...
    PHP Code:
    $database Database::selectDatabase($configuration);
    $connection $database->createConnection();
    $sql $database->createSqlBuilder();
    $result $connection->select($sql->select('people', array('name = "Fred"'))); 
    There is nothing database specifc in this code. And there are lot's of objects that can be passed around instead of $configuration.

    This stuff is how you move upward from writing scripts and simple applications into the realm of system architecture. That is big systems and frameworks. It's a fascinating area, but to get there you have a few steps to take:

    1) Improve your code quality. Have a read of "Refactoring" by Martin Fowler.
    2) Get used to object interfaces running the show. Learn some UML and unit testing.
    3) Then start on the patterns books, such as "Design Patterns" by Gamma, Helm, Vlissides and Johnson. Factory and Iterator are examples of patterns. Heavy stuff though.

    It'll take a year at least.

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  10. #10
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    Connecticut
    Posts
    173
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Exactly the sort of stuff I was looking for Marcus, thank you much

    The example isn't too overwhelming, but you're right that it will take a year or more before I could explain the same thing to someone else.

    /me starts coding.

  11. #11
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Quote Originally Posted by dreamscape
    But the point is, you cannot make structural change like that to a critical part of the system (I think the program config qualifies as highly critical) and not have it affect other parts of the program, no matter how "separated" all the parts are.
    There are two separate things here. If you pass the object/hash around then you can write adapters as I said. That allows you to have both systems present in your code. Or more if you use lot's of external libraries. It also means any adaption code is close to the objects that need it. It's very obvious in the code above that the format is changing before it's passed in. As most objects use very little configuration, the adapters can be pretty small.

    The problem you are describing is one of access, not the formats of the two different configurations. Changing from a global to a registry will cause problems for sure. There is not much that can be done about that except, as you say, to have two simultaneous access systems. But then that's the problem with the globalness

    Configuration does have a habit of "cross-cutting" the code, making localisation difficult. I certainly agree this tendancy can make things difficult. I think the "cure" is to pass down objects that are configured higher up.

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  12. #12
    SitePoint Wizard dreamscape's Avatar
    Join Date
    Aug 2005
    Posts
    1,080
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    >> The problem you are describing is one of access, not the formats of the two different configurations.

    No, actually it is both. Both the access and format have changed.

    As you say one could write an adapters, but IMO adapters should be limited to interaction with 3rd party stuff. I think if you find yourself with a bunch of adapters to "adapt" your code to your code, something is clearly amiss.

    For your own code, IMO, as I said I think it is better to temporarily keep both systems while you depreciate the old one (with the idea of dropping the old system at a certain application version).

    I suppose we're sort of saying the same thing, but I just really don't think using adapters to adapt your own code to your own code is the wisest thing to do. If you begin down that road, then what is to stop you from not ever having any sort of application API standards? This methodology seems to be saying, "to hell with the application API, we'll just write internal adapters anywhere we don't match up with ourselves." ... hmm, well personally that's not the road I want to be starting down. I prefer to say, "Here is the now preferred application API standard method for this, and here is a deprecated method that will be eventually removed at version X. Be sure we make all changes prior to this version."

  13. #13
    SitePoint Wizard dreamscape's Avatar
    Join Date
    Aug 2005
    Posts
    1,080
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    PS. dotDan, sorry for being a thread stealing whore. It's usually not my style.

  14. #14
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    Connecticut
    Posts
    173
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    It's alright, I've got more than enough help and the extra discussion is interesting food for thought.

  15. #15
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Quote Originally Posted by dreamscape
    I suppose we're sort of saying the same thing,
    We are, I am describing a transition period. It's just easier to manage without the global part. As you change each class, add the adapter to the parameters. That adapter gets promoted as the change spreads until it reaches the end and becomes unecessary, bit like a boundary marker. Much harder with a global item.

    You would only have permant adapters when you have some kind of contract across packages, as you say. An external library is agood example. Another is a section of the code that is subject to frequent changes and you want to wall off. For example a library that is needed by two competing parts of the application. You don't want changes in one part to affect the other, thus you have simpler facades on the library for each application.

    Another case is a longer term low priority refactoring. Sometime things can take a week or two to change and stuff comes up during the week. Suddenly you have a refactoring that could be half done for months, but you still have to keep the system running. While this happens you will have adapters around method calls. Still, again as you say, you don't want that kind of mess permanently or it will start to pile up.

    So we agree .

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  16. #16
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    Connecticut
    Posts
    173
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Here's what I have so far..

    dao.php
    PHP Code:
    <?php
        
    class DAO {
            function 
    DAO($config) {
                if(
    $config['dbtype'] == 'pgsql') {
                    if(
    $this->IsClass('pgsql')) {
                        
    $this->sql = new PgSQL($config);
                    } else {
                        
    $this->error('no_database');
                    }
                } elseif(
    $this->IsClass('mysql')) {
                    
    $this->sql = new MySQL($config);
                } else {
                    
    $this->Error('no_database');
                }
            }
            function 
    IsClass($class) {
                return 
    false;
                
    $this->className $class;
                
    $this->classFile 'packages/data/'.$class.'.php';
                if(
    file_exists($this->classFile)) {
                    include 
    $this->classFile;
                    if(
    class_exists($this->className)) {
                        return 
    true;
                    }
                }
            }
            function 
    Error($e) {
                
    # error handling
            
    }
        }
    ?>
    mysql.php
    PHP Code:
    <?php
        
    class MySQL extends DAO {
            function 
    MySQL($config) {
                
    $this->newConnection($config);
            }
            function 
    newConnection($config) {
                
    $this->connection mysql_connect($config['host'],$config['user'],$config['pass']);
                
    $this->selectDatabase($config);
            }
            function 
    selectDatabase($config) {
                
    $this->database mysql_select_db($config['db'],$this->connection);
            }
        }
    ?>
    Am I on the right track?

  17. #17
    SitePoint Wizard Young Twig's Avatar
    Join Date
    Dec 2003
    Location
    Albany, New York
    Posts
    1,355
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    You need to declare your class-scope variables up before your constructor. Something like this:

    PHP Code:
    <?php
        
    class MySQL extends DAO {
            var 
    $connection;
            var 
    $database;

            function 
    MySQL($config) {
                
    $this->newConnection($config);
            }
            function 
    newConnection($config) {
                
    $this->connection mysql_connect($config['host'],$config['user'],$config['pass']);
                
    $this->selectDatabase($config);
            }
            function 
    selectDatabase($config) {
                
    $this->database mysql_select_db($config['db'],$this->connection);
            }
        }
    ?>

  18. #18
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    Connecticut
    Posts
    173
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I always get an error when I do var $variable; and never could figure out why, so I abstained from doing it.


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •