db-torque-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Monroe <Greg.Mon...@dukece.com>
Subject RE: Change semantics of Criteria.add() and Criteria.or()
Date Mon, 12 Dec 2011 19:16:01 GMT
Moving Critia to another package is good.  I never thought it was a "util".

+1

Some thoughts on implementation:

Should we move to a "factory" model that allows for both old and new to exist?
E.g. Criteria becomes an abstract class and to get on you need to call 
Criterial.newCriteria();  With a setting that allows for getting old or new 
Versions of Criteria implimentations...

but this might be too messy with all method stubs  in the abstract and possibly 
requiring class casting in places.

Since we're talking about major changes, here are some thoughts on method names...

First, I think we've overloaded "add" too much.  There are something like 15 
add methods and 15+ addXXX methods.  These relate to widely different parts of 
the statement.. like addJoin(), addAlias(), etc.  I think it would be nice to 
refactor these to allow for easier identification and selection by using the 
query area they apply to.  So instead of "add" how about "where" as the base 
for WHERE clause section of a query.

Here's some example pseudo code:

/* SQL logical flags a.la SqlEnum used below */
Public Class SqlLogical {
  Public static final SqlLogical AND = new SqlLogical("AND");
  Public static final SqlLogical OR = new SqlLogical("OR");
  Public static final SqlLogical AND_NOT = new SqlLogica("AND NOT");
  Public static final SqlLogical OR_NOT = new SqlLogica("OR NOT");

/** 
 * Base condition method that all "convenience methods built on"
 *
 * @arg col Column object to use for comparison
 * @arg value Value to compare with
 * @arg comparison Type of comparison to make
 * @arg logical Logical conditions to use with clause (null = ADD)
 */
Public Criteria where( Column col, Object value, SqlEnum comparison, 
                       SqlLogical logical );

My preference for "convenience" where methods would be to just have 
type specific values and keep all the other options.  E.g.

Public Criteria where( Column col, int value, SqlEnum comparison, 
                       Boolean useOr, Boolean useNot );

This would cut way back on the number of methods in criteria by allowing
a lot of "combination" methods to be eliminated.

Probably the biggest complaint for this is the "readability" of it.  E.g.:

c.add(USERS.COL_ID, 1, SqlEnum.GREATER_THAN )
 .add(USERS.NAME, "admin%", SqlEnum.LIKE)
 .addNotIn(USERS.RIGHTS, rightsArray);
 
becomes
c.where(USERS.COL_ID, 1, SqlEnum.GREATER_THAN, null )
 .where(USERS.NAME, "%admin%", SqlEnum.LIKE, null ),
 .where(USERS.RIGHTS, rightsArray, SqlEnum.IN, SqlLogical.AND_NOT);

Personally, I like the simplicity of fewer methods to remember and the 
resulting easier editor "autoselection" entry.  But if we wanted to 
there could be more "where" convenience methods. 

Of course the criterion methods would be used to do nested conditions.
And if you wanted the .and().or().andNot() readability, you could do
the logicals using the Criterion classes.

The same naming convention could be used for things like:

addJoin() => join()
add...OrderByColumn => orderBy(column, direction)
addGroupBy => groupBy()

and the like..

Finally, if we don't / can't do a factory to switch syntaxes, then this
naming conventions would mean the old methods could stay with a deprecated
tag and the new methods would "stand out" as separate.

Just some thoughts on this.

Greg

-----Original Message-----
From: Thomas Vandahl [mailto:tv@apache.org] 
Sent: Monday, December 12, 2011 10:59 AM
To: Apache Torque Developers List
Subject: Re: Change semantics of Criteria.add() and Criteria.or()

Am 12.12.2011 15:07, schrieb Thomas Fox:
> 1) deprecate or remove the existing class and create a new one. Variants:
> 1a) move the new class to a new package

+1
While the danger of an automatic "Organize imports" still exists, it is 
probably the best solution.

> 2) deprecate the existing methods and create new ones. We could
> deprecate/remove the add methods and tell the user to use the and methods.
> But I do not have a good new name for the "or" methods.

Its not really a good solution but what about AND() and OR().

> 3) Because of TORQUE-168, new methods have been added which use a Column
> object as column name. Until now, Strings have been used as column name. We
> could deprecate/remove the methods which use String values. This means that
> people which do NOT use the constants from the Peer classes have more work.

+2
Yeah, but I'd prefer more work over the possibility of wrong code. Anytime.

Bye, Thomas.

---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org

DukeCE Privacy Statement:
Please be advised that this e-mail and any files transmitted with
it are confidential communication or may otherwise be privileged or
confidential and are intended solely for the individual or entity
to whom they are addressed. If you are not the intended recipient
you may not rely on the contents of this email or any attachments,
and we ask that you please not read, copy or retransmit this
communication, but reply to the sender and destroy the email, its
contents, and all copies thereof immediately. Any unauthorized
dissemination, distribution or copying of this communication is
strictly prohibited.

---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org


Mime
View raw message