Class properties dbpassword, dbname, dbhost and dbh are undefined in wpdb
|Reported by:||joelhardi||Owned by:||nacin|
The class members $dbuser, $dbpassword, $dbname, $dbhost and $dbh are set by wpdb's constructor but only one of these ($dbuser, since 2.9.0) is declared with a var statement.
I think these should be explicitly declared, both for basic documentation and to declare their visibility so developers know what they can use. I know in the past there have been issues with developers using supposedly private properties/methods, which then turns into a defacto API that is hard to deprecate. In this case, these properties aren't marked private and are just magically created in __construct without any documentation as far as I can tell.
Personally I don't care whether they are declared using var and marked as private or declared as private/protected. I would prefer explicit private but the rest of wpdb still looks PHP4 so I've used var statements in the attached patch.
What I propose is for these properties to be private, since they are passed at runtime to the constructor and immediately used to establish a database connection ($dbh). Subsequently changing the value of $wpdb->dbhost, for example, doesn't alter the existing database connection ($wpdb->db_connect() would have to be re-run to do that). Also, $dbuser is already labeled private so this is consistent.
Although private, I think these properties could/should have getters so that they're effectively read-only public. In particular, there needs to be a getter for the database connection handle $dbh (I'm a plugin author and I currently grab $wpdb->dbh to access the database) if it's going to be private. In the patch I've added a getter (get_db_connect(), but I don't really care what the method is called).
A little more background: In an analogous situation, previously I used $wpdb->prefix to retrieve the WordPress table prefix, but since 3.0 there is a getter, get_blog_prefix(), for this (and simply retrieving $wpdb->prefix isn't compatible with multisite, so that "old" API is broken). So this is why I suggest giving wpdb a clearer API. Other people may have suggestions, and if the task is blessed I could give wpdb a deeper look (another thing that might be helpful would be to add an interface). But I thought I'd submit this minor enhancement first.
Change History (19)
- Owner set to nacin
- Resolution set to fixed
- Status changed from new to closed
- Resolution fixed deleted
- Status changed from closed to reopened
- Keywords dev-feedback removed