You are viewing a plain text version of this content. The canonical link for it is here.
Posted to torque-dev@db.apache.org by Thomas Fox <Th...@seitenbau.net> on 2011/12/11 20:01:14 UTC
Change semantics of Criteria.add() and Criteria.or()
I propose to change the semantics of the Criteria.add() and Criteria.or()
methods.
In my opinion, both behave counter-intuitively. My opinion of an intuitive
behaviour would be:
- Criteria.or(someCondition) should result in a where clause which contains
((whatever was in the where condition before) OR someCondition)
- Criteria.add(someCondition) should result in a where clause which
contains ((whatever was in the where condition before) AND someCondition),
i.e. the same as Criteria.and(someCondition)
Instead, the behaviour is now
- Criteria.or(someCondition) only or's the new condition with any existing
condition(s) that affects the same column.
- Criteria.add(someCondition) replaces any existing condition(s) that
affects the same column with the new condition.
I propose to change the behaviour of the Criteria.add() and Criteria.or()
methods to the intuitive behaviour stated above in Torque 4. (I have seen
to many bugs at $work resulting from the counter-intuitive behaviour).
Any objections ?
Thomas
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
RE: Change semantics of Criteria.add() and Criteria.or()
Posted by Thomas Fox <Th...@seitenbau.net>.
Thanks a lot, Greg, for your suggestions.
Greg Monroe wrote:
> Some take it or leave it suggestions:
>
> We might still want to refactor to the Factory methodology
> even if there is only one class. This would make it easier
> for future versions to do Criteria changes without as much
> disruption or for application developers to add in their
> own (DB Type specific?) versions.
I'd rather not add complexity which is not needed. It will mean three
classes (interface, implementation and factory)
instead of one, and I honestly do not see a compelling use case.
> I'd suggest that the Column, ColumnImpl, and ColumnValue
> classes be moved into this new criteria package. AFAIK,
> they are only used by the Criteria class and putting them
> there will keep down confusion between ColumnMap and
> Column.
Hm, as both Database and Table are in the package where Column is now, I'd
rather leave it there.
Thomas
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
RE: Change semantics of Criteria.add() and Criteria.or()
Posted by Greg Monroe <Gr...@dukece.com>.
Sounds good. Eventually dropping the add methods but keeping
the convenience and/or methods will create a much cleaner
and easily understood structure.
Some take it or leave it suggestions:
We might still want to refactor to the Factory methodology
even if there is only one class. This would make it easier
for future versions to do Criteria changes without as much
disruption or for application developers to add in their
own (DB Type specific?) versions.
I'd suggest that the Column, ColumnImpl, and ColumnValue
classes be moved into this new criteria package. AFAIK,
they are only used by the Criteria class and putting them
there will keep down confusion between ColumnMap and
Column.
Greg
-----Original Message-----
From: Thomas Fox [mailto:Thomas.Fox@seitenbau.net]
Sent: Sunday, December 18, 2011 3:38 PM
To: Apache Torque Developers List
Subject: RE: Change semantics of Criteria.add() and Criteria.or()
Thanks, Greg and Thomas, for your feedback.
So my suggestion would be to create a new package,
org.apache,torque.criteria, and copy the Criteria class to the new package.
The old criteria class will stay as and where it is but become deprecated.
Probably there will be a deprecated interface CriteriaInterface which
collects a few Criteria methods, to reduce code duplications in the
SqlBuilder class. Most of the methods will not be in the interface as there
is a problem that most of the classes will return the class instance and
I'd rather not have the new Criteria class have methods whic return a
deprecated type.
The SqlEnum would also fit better in the new criteria package than in the
util package, so in my opinion it should be moved
My idea would be to remove the old util.Criteria class (and the
CriteriaInterface) in Torque 4.1
I like the name of the "where" method but I rather value readable code, so
I'd rather not remove the and or or methods. I'd rather suggest to leave
the and and or methods, but deprecate the add methods and offer them anew
as where methods. This results in very readable code.
criteria.where(AuthorPeer.ID, 1)
.and(AuthorPeer.name,'Bloch');
(this is of course discussable as the and and the where methods have the
same semantics so one could also argue that one of them is enough)
As for reducing the length of the Criteria method, I'd propose a few things
to do on the new Criteria class
- move the Join and Criterion inner classes to full-blown classes of their
own
- remove all methods which take Strings as column namens.
- remove methods which came from the Hashtable interface, such as size(),
getXXX(), clear() etc
Doing this will result in a Criteria class which is 1739 lines long
(including the deprecated add methods), most of them javadoc, less than the
2000 lines considered good style. I believe this size is ok for a class
with such a lot functionality as the Criteria class.
I'd not bother for factory methods as the plan is to remove the old
util.criteria class anyway.
Is this ok for you ? If yes I'd check the code in in a few days to have a
better basis of discussion.
Thomas
Greg Monroe wrote:
> 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 newto
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
>
---------------------------------------------------------------------
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
RE: Change semantics of Criteria.add() and Criteria.or()
Posted by Greg Monroe <Gr...@dukece.com>.
First, thank you Thomas F for keeping up moving forward!
+1 on Thomas V's alias column object.
No strong opinion on the flag issue.
-----Original Message-----
From: Thomas Vandahl [mailto:tv@apache.org]
Sent: Wednesday, December 28, 2011 8:43 AM
To: Apache Torque Developers List
Subject: Re: Change semantics of Criteria.add() and Criteria.or()
On 27.12.11 10:46, Thomas Fox wrote:
> On checking the test, I discovered that there is a "valid" use case for
> String column namnes even if one uses the generated code, namely aliases.
I don't know what exact use case you have in mind but I propose to
provide a method to create a Column object for aliases and remove the
string-based methods altogether.
> # creates methods which take the old org.apache.torque.util.Criteria object
> torque.use.util.criteria
> # creates methods which take the new org.apache.torque.criteria.Criteria
> object
> torque.use.criteria.criteria
>
> In my opinion, the default value of both options should be true. What do
> you think ?
IMHO, it the time for radical changes is now. So I suggest to support
only one flag to switch using either the old or the new class but not both.
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
Re: Change semantics of Criteria.add() and Criteria.or()
Posted by Thomas Fox <Th...@seitenbau.net>.
> Thomas Vandahl wrote:
> On 27.12.11 10:46, Thomas Fox wrote:
> > On checking the test, I discovered that there is a "valid" use case for
> > String column namnes even if one uses the generated code, namely
aliases.
>
> I don't know what exact use case you have in mind but I propose to
> provide a method to create a Column object for aliases
This method already exists: new org.apache.torque.ColumnImpl(String).
> and remove the
> string-based methods altogether.
I'd rather leave them in. Having methods which take a String object feels
"lightweight" and "simple" for users, and besides they do not add much
complexity (Basically calling new ColumnImpl(String) in each of them).
I do not have a very strong opinion here, if both Greg and you insist I'll
take them out.
Thomas
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
Re: Change semantics of Criteria.add() and Criteria.or()
Posted by Thomas Vandahl <tv...@apache.org>.
On 27.12.11 10:46, Thomas Fox wrote:
> On checking the test, I discovered that there is a "valid" use case for
> String column namnes even if one uses the generated code, namely aliases.
I don't know what exact use case you have in mind but I propose to
provide a method to create a Column object for aliases and remove the
string-based methods altogether.
> # creates methods which take the old org.apache.torque.util.Criteria object
> torque.use.util.criteria
> # creates methods which take the new org.apache.torque.criteria.Criteria
> object
> torque.use.criteria.criteria
>
> In my opinion, the default value of both options should be true. What do
> you think ?
IMHO, it the time for radical changes is now. So I suggest to support
only one flag to switch using either the old or the new class but not both.
Bye, Thomas.
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
Re: Change semantics of Criteria.add() and Criteria.or()
Posted by Thomas Fox <Th...@seitenbau.net>.
I just committed the new Criteria class, ready for inspection.
On checking the test, I discovered that there is a "valid" use case for
String column namnes even if one uses the generated code, namely aliases.
So I decided to leave the method staking String column names in the new
Criteria object. This leaves the new object larger than 2000 lines, but
once all deprecated methods are removed, it will be less than 2000 lines.
In my opinion, this is ok.
Please speak up now if you are of the opinion that the methods taking
string column names should be removed, this can still be done now.
Next step would be to change the templates to create methods which use the
new Criteria class. Up to now, only the old class is used so the new class
is pretty useless.
My current idea is to cretae two build options
# creates methods which take the old org.apache.torque.util.Criteria object
torque.use.util.criteria
# creates methods which take the new org.apache.torque.criteria.Criteria
object
torque.use.criteria.criteria
In my opinion, the default value of both options should be true. What do
you think ?
Thomas
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
Re: Change semantics of Criteria.add() and Criteria.or()
Posted by Thomas Vandahl <tv...@apache.org>.
On 18.12.11 21:37, Thomas Fox wrote:
> Is this ok for you ? If yes I'd check the code in in a few days to have a
> better basis of discussion.
Yeah, go ahead. I particularly like the idea of the where() method.
Bye, Thomas.
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
RE: Change semantics of Criteria.add() and Criteria.or()
Posted by Thomas Fox <Th...@seitenbau.net>.
Thanks, Greg and Thomas, for your feedback.
So my suggestion would be to create a new package,
org.apache,torque.criteria, and copy the Criteria class to the new package.
The old criteria class will stay as and where it is but become deprecated.
Probably there will be a deprecated interface CriteriaInterface which
collects a few Criteria methods, to reduce code duplications in the
SqlBuilder class. Most of the methods will not be in the interface as there
is a problem that most of the classes will return the class instance and
I'd rather not have the new Criteria class have methods whic return a
deprecated type.
The SqlEnum would also fit better in the new criteria package than in the
util package, so in my opinion it should be moved
My idea would be to remove the old util.Criteria class (and the
CriteriaInterface) in Torque 4.1
I like the name of the "where" method but I rather value readable code, so
I'd rather not remove the and or or methods. I'd rather suggest to leave
the and and or methods, but deprecate the add methods and offer them anew
as where methods. This results in very readable code.
criteria.where(AuthorPeer.ID, 1)
.and(AuthorPeer.name,'Bloch');
(this is of course discussable as the and and the where methods have the
same semantics so one could also argue that one of them is enough)
As for reducing the length of the Criteria method, I'd propose a few things
to do on the new Criteria class
- move the Join and Criterion inner classes to full-blown classes of their
own
- remove all methods which take Strings as column namens.
- remove methods which came from the Hashtable interface, such as size(),
getXXX(), clear() etc
Doing this will result in a Criteria class which is 1739 lines long
(including the deprecated add methods), most of them javadoc, less than the
2000 lines considered good style. I believe this size is ok for a class
with such a lot functionality as the Criteria class.
I'd not bother for factory methods as the plan is to remove the old
util.criteria class anyway.
Is this ok for you ? If yes I'd check the code in in a few days to have a
better basis of discussion.
Thomas
Greg Monroe wrote:
> 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 newto
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
>
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
RE: Change semantics of Criteria.add() and Criteria.or()
Posted by Greg Monroe <Gr...@dukece.com>.
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
Re: Change semantics of Criteria.add() and Criteria.or()
Posted by Thomas Vandahl <tv...@apache.org>.
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
Re: Change semantics of Criteria.add() and Criteria.or()
Posted by Thomas Fox <Th...@seitenbau.net>.
> > I propose to change the behaviour of the Criteria.add() and
Criteria.or()
> > methods to the intuitive behaviour stated above in Torque 4. (I have
seen
> > to many bugs at $work resulting from the counter-intuitive behaviour).
> > Any objections ?
>
> I wholeheartedly agree to your proposal. However this will break
> existing code. The old code would compile but do soemthing different. Do
> we have a warning sign big enough to show it to everybody? Or is there
> some way to create a method signature that leads to a compile error then?
Technically, we could do the following
1) deprecate or remove the existing class and create a new one. Variants:
1a) move the new class to a new package
1b) rename the class (but I do not have a good new name and many OR Mappers
use the name Criteria)
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.
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.
4) Major version change is warning sign enough
Personally I am not sure what to suggest. Criteria is a very big class and
removing methods is a good thing IMHO, so deprecating the add methods and
deprecating the methods which use Strings ( 2) + 3) ) should be ok as
warning signs. 4) is also fine with me. But I am open to other suggestions.
Thomas
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
Re: Change semantics of Criteria.add() and Criteria.or()
Posted by Thomas Vandahl <tv...@apache.org>.
Am 11.12.2011 20:01, schrieb Thomas Fox:
> I propose to change the behaviour of the Criteria.add() and Criteria.or()
> methods to the intuitive behaviour stated above in Torque 4. (I have seen
> to many bugs at $work resulting from the counter-intuitive behaviour).
> Any objections ?
I wholeheartedly agree to your proposal. However this will break
existing code. The old code would compile but do soemthing different. Do
we have a warning sign big enough to show it to everybody? Or is there
some way to create a method signature that leads to a compile error then?
Bye, Thomas.
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
Re: Nested queries (was: Re: Change semantics of Criteria.add() and Criteria.or())
Posted by Graham Leggett <mi...@sharp.fm>.
On 12 Dec 2011, at 2:02 PM, Thomas Fox wrote:
> Do you mean subselects ? This already works in Torque 3.3.
> E.g.
> Criteria.add(AUTHORPeer.ID, someNestedCriteria, Criteria.IN);
That's exactly what I meant, and wasn't aware of it, thanks for that :)
Regards,
Graham
--
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
Nested queries (was: Re: Change semantics of Criteria.add() and Criteria.or())
Posted by Thomas Fox <Th...@seitenbau.net>.
Graham Leggett <mi...@sharp.fm> wrote
> ...
>
> Is there any possibility of supporting nested queries? Something
> like Criteria.add(someNestedCriteria).
Do you mean subselects ? This already works in Torque 3.3.
E.g.
Criteria.add(AUTHORPeer.ID, someNestedCriteria, Criteria.IN);
Thomas
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
Re: Change semantics of Criteria.add() and Criteria.or()
Posted by Graham Leggett <mi...@sharp.fm>.
On 11 Dec 2011, at 9:01 PM, Thomas Fox wrote:
> I propose to change the semantics of the Criteria.add() and Criteria.or()
> methods.
>
> In my opinion, both behave counter-intuitively. My opinion of an intuitive
> behaviour would be:
> - Criteria.or(someCondition) should result in a where clause which contains
> ((whatever was in the where condition before) OR someCondition)
> - Criteria.add(someCondition) should result in a where clause which
> contains ((whatever was in the where condition before) AND someCondition),
> i.e. the same as Criteria.and(someCondition)
>
> Instead, the behaviour is now
> - Criteria.or(someCondition) only or's the new condition with any existing
> condition(s) that affects the same column.
> - Criteria.add(someCondition) replaces any existing condition(s) that
> affects the same column with the new condition.
>
> I propose to change the behaviour of the Criteria.add() and Criteria.or()
> methods to the intuitive behaviour stated above in Torque 4. (I have seen
> to many bugs at $work resulting from the counter-intuitive behaviour).
> Any objections ?
+1.
This has confused me in the past, simpler is better.
Is there any possibility of supporting nested queries? Something like Criteria.add(someNestedCriteria).
Regards,
Graham
--
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org