
Originally Posted by
Tony Marston
That is because I believe in the KISS principle. I do not like to make anything more complicated that it needs to be.

Originally Posted by
Tony Marston
I am not confusing the issue with subclasses. I am merely saying that I have a single User class, not a User and a UserMapper class.
By "encapsulating" everything in a single "User" class you have completly distorted the DAO pattern.
Here is your interface for a Person in your example application. This is not code writing by someone who believes in "Keeping it Simple Stupid" nor does it take into account massive amounts of Object Oriented Principles.
There is some serious refactoring to do.
PHP Code:
class Person extends Default_Table {
function Person();
function getFieldSpec_original ();
function getValRep ($item = 'star_sign');
function _cm_commonValidation ($fieldarray, $orignaldata);
function _cm_getExtraData ($where, $fieldarray);
function _cm_getForeignData ($fieldarray);
function _cm_getInitialData ($fieldarray);
function _cm_popupCall ($popupname, &$where);
function _cm_pre_getData ($where_str);
}
class Default_Table {
function Default_Table ();
function deleteRecord ($fieldarray);
function deleteSelection ($selection);
function formatData ($fieldarray);
function getClassName ();
function getCount ($where);
function getData ($where) ;
function getData_unformatted ($where);
function getDBname () ;
function getEnum ($fieldname) ;
function getErrors () ;
function getExpanded ();
function getExtraData ($input) ;
function getFieldArray () ;
function getFieldSpec () ;
function getFieldSpec_original () ;
function getInitialData ($where) ;
function getLastPage () ;
function getLookupData () ;
function getMessages () ;
function getNavButtons ($task_id) ;
function getNodeData ($expanded, $where=null);
function getNumRows () ;
function getOrderBy () ;
function getOrderBySeq () ;
function getPkeyArray ();
function getPkeyNames () ;
function getTableName () ;
function getValRep ($item, $where=null) ;
function getWhere () ;
function insertMultiple ($fieldarray) ;
function insertRecord ($fieldarray) ;
function popupCall ($popupname, $where, $script_vars) ;
function popupReturn ($fieldarray, $return_from, $selection);
function setAction ($action) ;
function setDefaultOrderBy ($sql_orderby) ;
function setFieldAccess () ;
function setFieldArray ($fieldarray) ;
function setOrderBy ($sql_orderby) ;
function setOrderBySeq ($sql_orderby_seq) ;
function setPageNo ($pageno = '1') ;
function setRowsPerPage ($rows_per_page) ;
function setSqlSearch ($sql_search) ;
function sqlAssemble ($where) ;
function updateLinkData ($fieldarray, $postarray) ;
function updateMultiple ($fieldarray, $postarray);
function updateRecord ($fieldarray) ;
function updateSelection ($replace, $selection);
function validateDelete ($fieldarray) ;
function validateInsertArray ($fieldarray) ;
function validateUpdateArray ($fieldarray) ;
function _cm_commonValidation ($fieldarray, $originaldata) ;
function _cm_deleteSelection ($selection) ;
function _cm_formatData ($fieldarray) ;
function _cm_getExtraData($where, $fieldarray) ;
function _cm_getForeignData ($fieldarray) ;
function _cm_getInitialData ($fieldarray) ;
function _cm_popupCall ($popupname, $where) ;
function _cm_popupReturn ($fieldarray, $return_from, $selection) ;
function _cm_post_deleteRecord ($fieldarray) ;
function _cm_post_getData ($rowdata, &$where) ;
function _cm_post_insertRecord ($fieldarray) ;
function _cm_post_updateMultiple ($fieldarray) ;
function _cm_post_updateRecord ($fieldarray, $old_data) ;
function _cm_pre_deleteRecord ($fieldarray) ;
function _cm_pre_getData ($where_str) ;
function _cm_pre_insertRecord ($fieldarray) ;
function _cm_pre_updateLinkdata ($fieldarray, &$postarray) ;
function _cm_pre_updateMultiple ($fieldarray) ;
function _cm_pre_updateRecord ($fieldarray) ;
function _cm_updateSelection ($replace, $selection);
function _cm_validateDelete ($fieldarray) ;
function _cm_validateInsert ($fieldarray) ;
function _cm_validateUpdate ($fieldarray, $originaldata) ;
function _dml_deleteRecord ($fieldarray);
function _dml_deleteRelations ($fieldarray);
function _dml_getCount ($where);
function _dml_getData ($where);
function _dml_getEnum ($item);
function _dml_insertRecord ($fieldarray);
function _dml_ReadBeforeUpdate ($where);
function _dml_updateRecord ($fieldarray, $oldarray);
function _dml_updateSelection ($selection, $replace);
function _dml_validateDelete ($fieldarray);
function _getDBMSengine ();
function __sleep () ;
}
Take a look at the common functions. Everything to do with meta data about an entity, put in some sort of TableMetaData object. Everthing to do with insert/update validation put in a Validation bject. Pass the validation object the TableMetaData object and the data to be validated. Everything to do with querying the database(setOrderBy(), setWhere() etc..) put in a query object and pass that object to the DAO.
Having 100 or so functions in a class is a serious bad code smell. Doing these things and refactoring will drop the amount of code in your architecture by a ton.

Originally Posted by
Tony Marston
You are arguning just because I dare to disagree with you. Just because I choose to read from a diffent design 'bible' you seem to think I am a heretic. I do not have DataAccessor or DataMapper classes for the simple reason that I do not follow that particular design methodology. I use the 3 tier architecture in which I have components in the presentation layer, the business layer, and the data access layer.
You are definately on the right track with everything in your architecture but there are blatent problems that could be addressed.
The 3 tiers in your architecture are mixed up as far as I can see.
Your Presentation is tied to your database through column names, and form names. You couldnt change your database without changing your presentation. You cant change your presentation without changing your database. I find the way you have done this to be extremely difficult to use.
You have actual sql in your controller that is passed into your "DAO". This means you cant change your database without changing your controller.

Originally Posted by
Tony Marston
I do not have 'flex points'. I have business logic in the business layer and data access logic in the data access layer. The purpose of a data access layer is so that I can switch databases whenever I wish. My code fulfills that purpose, therefore it cannot be wrong.
Agreed, you dont have flex point anywhere.
The purpose of a data access layer is to "encapsulate" communication with the data source. Again, having sql in your controller screws this up.

Originally Posted by
Tony Marston
Flex points do not appear in any of the design philosophies which I read, therefore I do not consider them to be a 'requirement'. I have written code which demonstrates the use of encapsulation, inheritance and polymorhism, and as these are the primary principles of OOP my code is therefore OO. It may not be your particular brand of OO, but that is irrelevant (IMHO).
Your code is not OO just because it demonstates the use of encapsulation, inheritance and polymorhism. Almost your entire architecture uses procedural functions (which isnt always a bad thing) and require_once style forwarding to abstracted "Transaction Controllers" that are procedural if/else scripts.
Why not make these transaction controllers classes? (Im refering to files like std.list1.inc). Ill bet you could refactor these alot.
Bookmarks