Bonesnap;11049915 wrote:It's amazing. One of the rare times I had very little hesitation to shell out the cash to buy a licence. Totally worth it.
I appreciate this ringing endorsement. I may do this when I get a chance to do some skill retooling. Eclipse PDT has been working for me lately.
Bonesnap;11049915 wrote:Anyway, I don't mean I love writing SQL so much I throw it all over the place. I just mean I rather write it then have some functions generate it for me, as I find it much easier to read it then to decipher a bunch of chained methods.
Agreed that chained methods are a bit off-putting at first. I'm still getting used to them. However, they do tend to cut way down on the need to perform string manipulation and escape values and stuff like that. I.e., it's easier to plug in your PHP values rather than writing something like this:
$sql = "INSERT INTO " . USERS_TABLE . " (user_id, username, user_regdate, user_password, user_email, user_icq, user_website, user_occ, user_from, user_interests, user_sig, user_sig_bbcode_uid, user_avatar, user_avatar_type, user_viewemail, user_aim, user_yim, user_msnm, user_attachsig, user_allowsmile, user_allowhtml, user_allowbbcode, user_allow_viewonline, user_notify, user_notify_pm, user_popup_pm, user_timezone, user_dateformat, user_lang, user_style, user_level, user_allow_pm, user_active, user_actkey, user_email_html, first_name, last_name, gender, date_of_birth, postal_code, country, occ_status, show_location, show_occupation, policy_check, license_code, on_register_action, on_register_redirect)
VALUES ($user_id,
'" . mysql_real_escape_string($reg_data_1['username']) . "',
" . time() . ",
'" . md5($reg_data_1['user_password']) . "',
'" . mysql_real_escape_string($reg_data_1['user_email']) . "',
'',
'',
'" . mysql_real_escape_string($user_occ) . "',
'" . mysql_real_escape_string($user_from) . "',
'',
'',
'',
'',
'',
" . $reg_data_1['viewemail'] . ",
'',
'',
'',
" . $reg_data_1['attachsig'] . ",
" . $reg_data_1['allowsmilies'] . ",
" . $reg_data_1['allowhtml'] . ",
" . $reg_data_1['allowbbcode'] . ",
" . $reg_data_1['allowviewonline'] . ",
" . $reg_data_1['notifyreply'] . ",
" . $reg_data_1['notifypm'] . ",
" . $reg_data_1['popup_pm'] . ",
" . $reg_data_1['user_timezone'] . ",
'" . mysql_real_escape_string($reg_data_1['user_dateformat']) . "',
'" . mysql_real_escape_string($reg_data_1['user_lang']) . "',
" . $reg_data_1['user_style'] . ",
0,
1, ";
That's an actual example of some of my oldest code. As you can see, now that MySQL has been deprecated, it's a real pain in the ass to update that code to use something more modern. And I have tons of it. It's gotta be done though. :queasy:
Bonesnap;11049915 wrote:And regarding your example, wouldn't fetch_by_id($id) need to write some SQL? I mean at some point, somewhere, something is going to have to write SQL, unless you have all your SQL generated. Perhaps a compromise would be to implement an approach to what you are suggesting with DB_my_table::fetch_by_id($id) but that function just has handwritten SQL, which I would definitely prefer.
I could have done it that way, but I did not. I have written a base class, SNEAKY_data_object. It looks something like this:
abstract class SNEAKY_data_object {
/**
* The CodeIgniter database object we will use to store and
* fetch these records.
* @var CI_DB_driver
*/
protected $_db;
/**
* Constructor.
* @param CI_DB_driver $db The CodeIgniter database object we'll use to save objects to db
* @param array $arr Optional associative array containing initial values
*/
public function __construct($db, $arr = NULL)
{
$this->_db = $db;
// import the values from $arr to the public properties defined in whatever class extends this one
if (is_array($arr)) {
foreach ($arr as $key => $value) {
if (property_exists($this, $key)) {
$this->$key = $value;
}
}
}
}
/**
* Makes sure that the called class has defined the constant PRIMARY_KEY_COLUMN
* @throws Exception if called class had not defined a primary key
*/
public static function check_for_primary_key()
{
$const_name = get_called_class() . "::PRIMARY_KEY_COLUMN";
if (!defined($const_name)) {
throw new Exception("$const_name is not defined");
}
}
/**
* Fetches a record from the database and instantiates it.
* @param CI_DB_driver $db CodeIgniter database object to query the db
* @param mixed $pkey The primary key value of the record to be fetched
* @return MPLN_data_object
*/
public static function fetch_by_pkey($db, $pkey)
{
self::check_for_primary_key();
$query = $db->get_where(static::DATABASE_TABLE, array(static::PRIMARY_KEY_COLUMN => $pkey));
if ($query->num_rows() < 1) {
return NULL; // nothing found
}
// TODO: warning if more than one?
$record = $query->row_array();
return new static($db, $record, $language);
}
} // class SNEAKY_data_object
There is only one class like this. It is an abstract class (meaning it is never itself instantiated).
I have another script (which I don't plan to share) which will examine every table in a database and automatically generate a DB_* class for that table that extends my SNEAKY_data_object class. Strictly speaking, I haven't written any SQL at all (it's generated by the CodeIgniter db object with lines like this one):
$query = $db->get_where(static::DATABASE_TABLE, array(static::PRIMARY_KEY_COLUMN => $pkey));
The generated classes all have a variety of methods, but here's a super simple version of one:
/**
* Class auto-generated from db table role
*/
class DB_role extends SNEAKY_data_object {
/**
* The database table's name
*/
const DATABASE_TABLE = "role";
/**
* They name of the table's primary key column
* @var string
*/
const PRIMARY_KEY_COLUMN = "id";
/**
* id column
* @var string
*/
const COL_ID = "id";
/**
* role_name column
* @var string
*/
const COL_ROLE_NAME = "role_name";
/**
* DB column id is of type smallint
* The primary key of db table role
* max length is 5
* @var int
*/
public $id;
/**
* DB column role_name is of type varchar
* max length is 50
* @var string
*/
public $role_name;
/**
*
* Fetches a DB_role object from the database
* @param CI_DB_driver $db CodeIgniter database object to query the db
* @param mixed $pkey The primary key of the record to be fetched
* @return DB_role
*/
public static function fetch_by_pkey($db, $pkey)
{
return parent::fetch_by_pkey($db, $pkey);
}
} // class DB_role
I didn't have to write that class either. It was automatically generated by another PHP script. If I want to change it I can. I don't usually have to.
Bonesnap;11049915 wrote:I think you can write SQL code and isolate the top-level logic of your application. Just write the SQL code in a deeper layer.
Yes this is true. One layer deeper is often enough.
Bonesnap;11049915 wrote:Indeed, what you are suggesting here is a better approach. Making one connection versus many sounds like a better idea. And I am definitely only working with a single database.
That sounds like a common situation, so your code will likely be reusable in many different projects.
Bonesnap;11049915 wrote:I have to admit that I do enjoy writing SQL statements
To each their own...
Bonesnap;11049915 wrote:I think this is at least part of the confusion and disconnect. I think I was confusing the two at times and they were being muddled in the replies back and forth. While I still don't think I'll be using anything other than MySQL, I am much more receptive to an OO data access approach where the data source can be swapped out but still writing SQL manually in a layer that's appropriate for it. I just don't want SQL auto-generated for me. Maybe an exception can be made for some very, very basic tasks, but generally speaking I'd like to keep the SQL manual.
Well it's probably going to be YOU maintaining the code so it's most important that you like it and understand it. I offer my advice here not because you need to take it -- on the contrary! You should be skeptical. I offer it because I feel like it has made me more productive.
Bonesnap;11049915 wrote:Sorry if I misinterpreted, but it seemed like you implied JOINs were an exception to the approach you were talking about. I guess they are just done on a case-by-base basis but I foresee them being common.
You can always pull out a JOIN if you need to. The CodeIgniter query builder object has features to help build them. Or you can just write raw SQL if you want. As to which class you put them in, that's up to you. I generally try to keep my DB_* classes (which refer only to one table) separate from more complex classes that might JOIN more than one table.
Bonesnap;11049915 wrote:How would you handle classes that interact with the database but have no representation in it? I think this may also be contributing to the confusion. The Region class I have is a very good example of this. There is no Region table in the database. It's just a class that handles functionality regarding regions (countries, continents, states, etc.). It's basically a logical grouping of methods. Some of those methods will need to read from the database (like get_countries_with_airports()). Other classes will have a representation in the database, like Airport. The Airport class will have a constructor that will create an object. It will also have methods that interact with the database (and probably some that won't).
I would just create some other class that starts with SNEAKY or something instead of DB* to suggest that it's not a simple table-specific class but rather something more complex. Then I would write anything I need in it: JOINS, record fetching, whatever.
