[jira] Commented: (TORQUE-22) Enhanced Database Mapping info

["Thomas Fischer (JIRA)" <ji...@apache.org>]

    [ 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: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org