[
http://issues.apache.org/jira/browse/TORQUE-22?page=comments#action_12412977 ]
Thomas Fischer commented on TORQUE-22:
--------------------------------------
In my opinion, the patch should be added to Torque. The one general thing I'm
not convinced about is the initialize() method in the DatabaseMap. It creates a
link between the Torque runtime and the generated classes (which is hidden by
using reflection). Why not call the XXXMapInit.init() class directly ? If the
name of the database is not known at compile time, the user may still use
reflection to access it.
Also, I came across a number of smallish issues:
Code style:
- Please add imports after the licence comments (e.g. ColumnMap).
- Please use a separate line for opening and closing braces.
- Please respect the 80 columns maximum width of java files (e.g.
InheritanceMap)
ColumnMap.java
- Could you please add the comments to the private fields as javadocs ? (also
TableMap.java)
- Why do you need the useInheritance mapping ? should not rather the
inheritanceType field be used to retrieve that information ?
- This may be a question of style, but I'd prefer the inheritanceMaps be
initialized when it is defined, not in the constructor, because all the other
fields are also initialized where they are defined.
- If there is no strong reason for doing otherwise, I'd find it more intuitive
if the field in the database map would be named like the corresponding
shema-xml attribute (inheritanceType-> inheritance, boolean usePrimitive ->
String javaType). Although the javaType has just two values now, maybe in the
future there might be more.
- setInheritance(). If this method is retained (see above), why does it set the
table's inheritance ? if this is necessary, it should at least be documented in
the javadocs.
TableMap.java
- Is the useInheritance field needed ? One could also ask the columns if their
inheritance map is empty. I do not like to keep the same information in
different places, becasue then synchronizing the two is always an issue. (one
might forget in later code changes that if one field is changed, the other
needs to be changed also)
- why do you add the deprecated method addColumn(String name, Object type,
boolean pk, String fkTable, String fkColumn, int size, int scale, String
javaName) ?
Peer.vm:
- The call of getMapBuilder() in getTableMap() should not be necessary if
Torque is initialized. Why did you add it ? Does it work if Torque is not
initialized ?
MapBuilder.vm
- Why is the initClass method needed ? Better not use reflection :
${className}.class is the thing you want.
- I'd rather remove the PEER_CLASS, OM_CLASS and MANAGER_CLASS class variables.
The values are needed only once and can be computed in local variables.
- Why do you use ($useManagers && $table.PrimaryKey.size() > 0) instead of
($useManagers) ? (also in Control.vm)
- Why do you generate the method getColumnName( String name ) for each class
instead of putting the method in the base class ? Also, I'd rather rename this
method to normalizeColumnName() or similar.
> Enhanced Database Mapping info
> ------------------------------
>
> Key: TORQUE-22
> URL: http://issues.apache.org/jira/browse/TORQUE-22
> Project: Torque
> Type: Improvement
> Components: Runtime, Generator, Test Project
> Versions: 3.2.1
> Reporter: CG Monroe
> Fix For: 3.2.1
> Attachments: MapEnhancements-ChangeLog.txt,
> MapEnhancements-ObjectWithManager.zip, MapEnhancements.zip
>
> The attached zip file makes the Database Map information much more robust and
> useful. Basic features added:
> If an application needs a fully populated Database Map structures, this can
> be done via the DatabaseMap.initialize() method.
> *Map classes will now preserve XML order and store many more XML attritbutes,
> including Inheritance information.
> MapBuilder template re-done to be more readable and expandable.
> The TableMap object can be used to get the associated Peer, OM, and Manager
> (if applicable) Class objects.
> Access to TableMap structures made easier in Peer ( existing method changed
> to public) and OM classes. (The theory here is that TableMap should know all
> this info and there should be an easy way to get there).
> Test-project now has DatabaseMap test suite that tests most of these features.
> Some misc Javadoc/synchronizing changes.
> Has been tested with aliased tables, beans, managers, DB schema's with
> different packages during development (test suite doesn't test all of these
> combinations yet..)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
