This is continuing the discussion I had with Greg Monroe on creating an
interface to access Criteria and Criteria.Criterion from the other
Torque classes. See
http://issues.apache.org/scarab/issues/id/TRQS341
for Greg's suggestion and previous discussion
IMHO, it is a good idea to have a well define interfaces (or an abstract
class) between major areas of responsiblity. In the long run, this helps
keep the areas from stepping on each other and helps in testing,
debugging, and enhancing the areas separate from each other.
This is certainly a good point, I agree to that. But now it comes to what
areas of responsibilities we want to define by the interfaces you
suggested. I can see two main ideas in your CriteriaInterface. The first
is "1) A provider of information for composing an SQL Query", the second
is "2) An object defining the contents of an SQL insert/update/delete".
Here are the methods that do not fit into 1):
CriteriaInterface.getNewCriterion(), CriteriaInterface.setLimit(),
CriteriaInterface.setOffset(), CriteriaInterface.remove(),
CriteriaInterface.add(), CriteriaInterface.addSelectColumn()
The setLimit() and setOffset() methods are only used by LargeSelect(),
they also do not fit into the pattern 2).
Then there are methods which are just convenience methods:
CriteriaInterface.getColumnName(), CriteriaInterface.getComparison(),
CriteriaInterface.getTableName(),CriteriaInterface.getValue()
The methods
CriteriaInterface.getFloat(), CriteriaInterface.getDouble()...
are both convenience methods and are an overshot at the only place they
are used (VillageUtils.setVillageValue), a simple cast would be enough
there.
Then, there are methods you have marked yourself as deprecated and are not
used anymore:
CriteriaInterface.getJoinL(), CriteriaInterface.getJoinR()
The equals() method is defined by any java Object and does not need to be
included in an interface declaration.
And, to make the list complete, including something I have said before:
The "extends Map" in the CriteriaInterface exposes an implementation
detail and should be avoided (I do hope we do not use it anywhere).
So my suggestion would be to split the CriteriaInterface in two
interfaces: QueryDataProvider(used for selects and deletes) and
UpdateDataProvider(used for updates and inserts), and remove the methods I
have mentioned above which are cluttering up the interface. The idea
between splitting is that we would have reasonably small interfaces with
well defined uses. Also, we would be to be able to use a separate object
for updates/inserts at some point.
If this seems reasonable, then we would also have to create two Criterion
Interfaces: QueryDataConstraint and UpdateDataElement.
The Criterion Interface has more methods which are needed by other Torque
classes but do not fit into the above pattern. This is because the
Criterion still renders itself in SQL. If this is factored out (as was
already done in the Criteria class), these methods can be removed. My
suggestion would be to move the SQL rendering code to
the SQLBuilder class.
Because this would also affect the interface to be created, I would
suggest to make Criterion a static inner class of Criteria. The reference
from Criterion to its enclosing Criterion object is that it uses the
Criteria's DB object, and as a default of the ignoreCase setting if no
specific ignoreCase is set for the criterion. Both is only neede for SQL
rendering, and could be accessed there by providing the enclosing Criteria
as additional argument.
In my opinion, Greg's suggestion plus the changes outlined above would
clear up the internal structure of the Criteria mechanism a lot. Any
opinions ?
Thomas
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
