propelorm/Propel2 — Issue #36: [RFC] Where to put the contents of the Peer classes?
Author: fzaninotto
Created: 2011-11-01 14:40:14+00:00
State: open

One of the goals expressed in #1 is to remove the Peer classeS. But where do we put the current content of the Peer classes then?

As a reminder, here is what the Peer classes store:

* Database mapping info, sometimes overlapping that of TableMap classes (constants like TABLE_NAME, column name constants), sometimes not (constants like DATABASE_NAME, number of columns and of lazy columns).
* References to the ActiveRecord and TableMap classes
* Instance pool data and methods
* Field names conversion data and methods
* doXXX queries, (mostly) overlapping methods generated in the ActiveQuery class
* Hydration utilities (`populateObject()`, `getPrimaryKeyFromRow()`, etc.)

Here are a few considerations:

**Mapping Info**: The generated ActiveRecord and ActiveQuery classes are both first-class citizens in Propel, and none is more legitimate than the other to store mapping data. The TableMap classes are, but they can't be overridden by the user, contrary to the current constants. I suggest to put all the mapping info into the TableMap classes, add a constant TABLE_MAP_CLASS to both the generated AR and AQ classes, that the user can override in stub classes to get different mapping than what was done at buildtime. I also suggest that all TableMap classes become singletons, since there is no "primary" way of getting/storing them - so we would need something like `BookTableMap::getinstance()`

**Hydration utilities**: This is related to data mapping to my opinion, so I'd move these to the TableMap classes as well.

**Instance Pool**: This could all go into one single InstancePool service class. But the question is: how could the AR and AQ classes access this service? Not to mention that the Instance Pool service needs access to methods like `getPrimaryKeyFromRow()`. I suggest to initialize an instance pool in the TableMap's `__construct()` methods, and store the instances in a static attribute of the TableMap class.

**doXXX Queries**: Remove them altogether. This will require some work (all the queries are not migrated to AQ classes, see for instance `AQ::delete()`), but it makes sense.

All in all, I suggest to "boost" the generated TableMap classes instead of putting reusable services (like Hydrators, Instance Pool, name converters, etc) in a central "EntityManager" object that would be required for every operation. I think it's better to make all models relatively independent, and let them only rely on a global registry for configuration (and, for instance, to get the default connection for queries). Also, generated methods are more powerful and require less overhead at runtime - that's the Propel way.

Thoughts?