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 Greg Monroe <Gr...@DukeCE.com> on 2006/12/05 20:09:38 UTC
Torque 4.0 - Add delete() / loadByPk(...) methods to Record object
I've often thought it would be nice for the
generated record object to have a delete()
and loadByPk() methods. It would help
simplify a lot of code.
Of course the loadByPK method would only be
added if there is a primary key. And if there
are multiple primary keys, the method sig. would
be something like:
loadByPk( <colType> key1, <colType> key2...)
throw NoRowsException, TooManyRowsException
and the delete() method would throw an exception
if called on a new record.
Greg Monroe <Mo...@DukeCE.com> (919)680-5050
C&IS Solutions Team Lead
Duke Corporate Education, Inc.
333 Liggett St.
Durham, NC 27701
Duke CE 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.
Re: Torque 4.0 - Add delete() / loadByPk(...) methods to Record object
Posted by Thomas Vandahl <tv...@apache.org>.
Greg Monroe wrote:
> and the delete() method would throw an exception
> if called on a new record.
+1 on this one. I added them myself.
Maybe one should setNew(true) after successful delete.
Bye, Thomas.
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
Re: Torque 4.0 - Add delete() / loadByPk(...) methods to Record object
Posted by Thomas Vandahl <th...@tewisoft.de>.
Greg Monroe wrote:
> Or do we just drop the do*( Record ) methods totally?
+1 for this. Reason: I'm always advocating readable code. It should be
easy for a developer to see what happens if a certain method is called.
Implicit features such as different behaviour depending on the existence
of a primary key is always evil.
Bye, Thomas.
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
RE: Torque 4.0 - Add delete() / loadByPk(...) methods to Record object
Posted by Greg Monroe <Gr...@DukeCE.com>.
One reason to track field state this is to make sure
the current Peer.doSelect/Delete/Insert( Record ) methods
will work as expected. As I said, currently default and
null values get add into these if you don't set the
primary key. E.g., with the current code base, the code:
User u = new User();
u.setUserId("foo");
List results = UserPeer.doSelect(u);
will ONLY return records where userId=foo and ALL
other non-primary fields are equal to the defaults or
null values. IHMO, the expected behaviour would be
to return all records with userId = foo.
Tracking what has been set would allow this broken
behaviour to be corrected.
Or do we just drop the do*( Record ) methods totally?
> -----Original Message-----
> From: Thomas Fischer [mailto:tfischer@apache.org]
> Sent: Thursday, December 14, 2006 2:52 PM
> To: Apache Torque Developers List
> Subject: RE: Torque 4.0 - Add delete() / loadByPk(...)
> methods to Record object
>
> Hm not so sure about the need to track field values. It makes
> things more
> complicated. I'd have no problems with that if it brings new
> features, but
> I can not see why
>
> User u = new User();
> u.setUserId("foo");
> u.setPassword("secret");
> u.load();
>
> should be better than
>
> Criteria criteria = new Criteria();
> criteria.add(UserPeer.USER_ID, "foo");
> criteria.add(UserPeer.PASSWORD, "secret");
> criteria.setSingleRecord(true);
> User u = (User) UserPeer.doSelect(criteria).get(0);
>
> Of course, for the given example, your code would be a bit
> shorter, but
> the criteria code is far more general. I do not believe in
> introducing
> several equivalent ways to do the same thing.
>
> I'd not object to tracking field values if there were other
> benefits, but
> I can see none at the moment. One could have the idea of
> improving save performance by saving only the modified
> fields, but the
> dbcp does prepared statement caching which improves
> performance if you
> always use the same prepared statement for saving, so its
> unclear whether
> this would actually improve performance.
>
> If you can see other benefits of field stat tracking, I'd be
> happy to hear
> them.
>
> Thomas
>
> On Thu, 14 Dec 2006, Greg Monroe wrote:
>
> > How about this:
> >
> > Assuming we add a mechanism to determine if a field
> > value has been set (e.g. via being loaded from the DB
> > or set by application code), have just a method like:
> >
> > load();
> >
> > The rules for load would work like this:
> >
> > If the record has a primary key(s) defined and the
> > object has this set, the object will be loaded (or reloaded)
> > from the DB.
> >
> > If there is no primary key or one is not set, it will
> > attempt to find a single record that matches all of the
> > "set" fields. If there is only one match, it load the
> > object. If there is more than one, it throws an exception.
> >
> > This would be similar to the current Peer methods that
> > take an record object and build criteria from it.
> >
> > The nice thing about this is that if you have tables
> > with other unique attributes, e.g. say a users table with
> > a numeric PK, but the userId is also unique. You can
> > do code like:
> >
> > User u = new User();
> > u.setUserId("foo");
> > u.setPassword("secret");
> > u.load();
> >
> > or
> >
> > User u = new User();
> > u.setId(123);
> > u.load();
> >
> > or
> >
> > User u = new User();
> > u.setEmail("foo@bar.org");
> > u.load();
> >
> > Depending on your needs. And if you have an existing
> > object and need to ensure the values match the database
> > one, just do:
> >
> > rec.load();
> >
> > FWIW, I started with the assumption about tracking field
> > level state because of the problems that can occure if
> > there are default values. IMHO, default values make the
> > current way to find things using the record object methods
> > very unreliable. The defaults get added into the Criteria
> > that is built and so you may not get what you expected
> > using these methods.
> >
> > Thus the need for tracking "set" values vs defaults. Even
> > with true support for determining NULL values is not enough
> > because the field value you want to search for might be NULL.
> > E.g., if I have a table with 10 fields that have default
> > values and allow NULLS, and do something like:
> >
> > RecordOm rec = new RecordOm();
> > rec.setField1(value1);
> > rec.setField2(value2);
> > rec.setField3(null);
> > List results = RecordOmPeer.doSelect(rec);
> >
> > I should get records with field1=value1, field2=value2, and
> > field3=NULL and no more.
> >
> > A way to do this would be to only allow the field storage
> > objects be accessed via "set" methods. Preferrably a single
> > set method for each fields (or a common setByName.... type
> > method). I.e. this.privateField= ... should be discouraged
> > internally in the generated objects. We should always use
> > the setField(..) methods unless absolutely needed.
> >
> > This way, anytime a value is set, a field state flag could
> > be set as well. This would allow for a method like:
> >
> > isModified( columnName );
> >
> > Note that default values would be set by object constructors
> > and not set the field state. This way a record object can be
> > examined to determine what fields need to be used in creating
> > a matching Criteria.
> >
> >> -----Original Message-----
> >> From: Thomas Fischer [mailto:tfischer@apache.org]
> >> Sent: Thursday, December 14, 2006 10:10 AM
> >> To: Apache Torque Developers List
> >> Subject: Re: Torque 4.0 - Add delete() / loadByPk(...)
> >> methods to Record object
> >>
> >> +1 on both. Would it make sense to rename the loadByPk()
> >> method reload() ?
> >> If yes, it could make sense to generate the method also if no
> >> pk exists but then throwing an exception.
> >>
> >> I also like Thoams' idea of sessing isNew to triue after the
> >> object has been deleted.
> >>
> >> Do you mind adding this to the wiki page ?
> >> http://wiki.apache.org/db-torque/NextRelease
> >>
> >> Thomas
> >>
> >> On Tue, 5 Dec 2006, Greg Monroe wrote:
> >>
> >>> I've often thought it would be nice for the generated
> >> record object to
> >>> have a delete() and loadByPk() methods. It would help
> >> simplify a lot
> >>> of code.
> >>>
> >>> Of course the loadByPK method would only be added if there is a
> >>> primary key. And if there are multiple primary keys, the
> >> method sig.
> >>> would be something like:
> >>>
> >>> loadByPk( <colType> key1, <colType> key2...)
> >>> throw NoRowsException, TooManyRowsException
> >>>
> >>> and the delete() method would throw an exception if
> called on a new
> >>> record.
> >>> Greg Monroe <Mo...@DukeCE.com> (919)680-5050 C&IS
> >> Solutions Team Lead
> >>> Duke Corporate Education, Inc.
> >>> 333 Liggett St.
> >>> Durham, NC 27701
> >>>
> >>>
> >>>
> >>>
> >>> Duke CE 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
> >
> >
>
> ---------------------------------------------------------------------
> 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: Torque 4.0 - Add delete() / loadByPk(...) methods to Record
object
Posted by Thomas Fischer <tf...@apache.org>.
Hm not so sure about the need to track field values. It makes things more
complicated. I'd have no problems with that if it brings new features, but
I can not see why
User u = new User();
u.setUserId("foo");
u.setPassword("secret");
u.load();
should be better than
Criteria criteria = new Criteria();
criteria.add(UserPeer.USER_ID, "foo");
criteria.add(UserPeer.PASSWORD, "secret");
criteria.setSingleRecord(true);
User u = (User) UserPeer.doSelect(criteria).get(0);
Of course, for the given example, your code would be a bit shorter, but
the criteria code is far more general. I do not believe in introducing
several equivalent ways to do the same thing.
I'd not object to tracking field values if there were other benefits, but
I can see none at the moment. One could have the idea of
improving save performance by saving only the modified fields, but the
dbcp does prepared statement caching which improves performance if you
always use the same prepared statement for saving, so its unclear whether
this would actually improve performance.
If you can see other benefits of field stat tracking, I'd be happy to hear
them.
Thomas
On Thu, 14 Dec 2006, Greg Monroe wrote:
> How about this:
>
> Assuming we add a mechanism to determine if a field
> value has been set (e.g. via being loaded from the DB
> or set by application code), have just a method like:
>
> load();
>
> The rules for load would work like this:
>
> If the record has a primary key(s) defined and the
> object has this set, the object will be loaded (or reloaded)
> from the DB.
>
> If there is no primary key or one is not set, it will
> attempt to find a single record that matches all of the
> "set" fields. If there is only one match, it load the
> object. If there is more than one, it throws an exception.
>
> This would be similar to the current Peer methods that
> take an record object and build criteria from it.
>
> The nice thing about this is that if you have tables
> with other unique attributes, e.g. say a users table with
> a numeric PK, but the userId is also unique. You can
> do code like:
>
> User u = new User();
> u.setUserId("foo");
> u.setPassword("secret");
> u.load();
>
> or
>
> User u = new User();
> u.setId(123);
> u.load();
>
> or
>
> User u = new User();
> u.setEmail("foo@bar.org");
> u.load();
>
> Depending on your needs. And if you have an existing
> object and need to ensure the values match the database
> one, just do:
>
> rec.load();
>
> FWIW, I started with the assumption about tracking field
> level state because of the problems that can occure if
> there are default values. IMHO, default values make the
> current way to find things using the record object methods
> very unreliable. The defaults get added into the Criteria
> that is built and so you may not get what you expected
> using these methods.
>
> Thus the need for tracking "set" values vs defaults. Even
> with true support for determining NULL values is not enough
> because the field value you want to search for might be NULL.
> E.g., if I have a table with 10 fields that have default
> values and allow NULLS, and do something like:
>
> RecordOm rec = new RecordOm();
> rec.setField1(value1);
> rec.setField2(value2);
> rec.setField3(null);
> List results = RecordOmPeer.doSelect(rec);
>
> I should get records with field1=value1, field2=value2, and
> field3=NULL and no more.
>
> A way to do this would be to only allow the field storage
> objects be accessed via "set" methods. Preferrably a single
> set method for each fields (or a common setByName.... type
> method). I.e. this.privateField= ... should be discouraged
> internally in the generated objects. We should always use
> the setField(..) methods unless absolutely needed.
>
> This way, anytime a value is set, a field state flag could
> be set as well. This would allow for a method like:
>
> isModified( columnName );
>
> Note that default values would be set by object constructors
> and not set the field state. This way a record object can be
> examined to determine what fields need to be used in creating
> a matching Criteria.
>
>> -----Original Message-----
>> From: Thomas Fischer [mailto:tfischer@apache.org]
>> Sent: Thursday, December 14, 2006 10:10 AM
>> To: Apache Torque Developers List
>> Subject: Re: Torque 4.0 - Add delete() / loadByPk(...)
>> methods to Record object
>>
>> +1 on both. Would it make sense to rename the loadByPk()
>> method reload() ?
>> If yes, it could make sense to generate the method also if no
>> pk exists but then throwing an exception.
>>
>> I also like Thoams' idea of sessing isNew to triue after the
>> object has been deleted.
>>
>> Do you mind adding this to the wiki page ?
>> http://wiki.apache.org/db-torque/NextRelease
>>
>> Thomas
>>
>> On Tue, 5 Dec 2006, Greg Monroe wrote:
>>
>>> I've often thought it would be nice for the generated
>> record object to
>>> have a delete() and loadByPk() methods. It would help
>> simplify a lot
>>> of code.
>>>
>>> Of course the loadByPK method would only be added if there is a
>>> primary key. And if there are multiple primary keys, the
>> method sig.
>>> would be something like:
>>>
>>> loadByPk( <colType> key1, <colType> key2...)
>>> throw NoRowsException, TooManyRowsException
>>>
>>> and the delete() method would throw an exception if called on a new
>>> record.
>>> Greg Monroe <Mo...@DukeCE.com> (919)680-5050 C&IS
>> Solutions Team Lead
>>> Duke Corporate Education, Inc.
>>> 333 Liggett St.
>>> Durham, NC 27701
>>>
>>>
>>>
>>>
>>> Duke CE 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
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
RE: Torque 4.0 - Add delete() / loadByPk(...) methods to Record object
Posted by Greg Monroe <Gr...@DukeCE.com>.
How about this:
Assuming we add a mechanism to determine if a field
value has been set (e.g. via being loaded from the DB
or set by application code), have just a method like:
load();
The rules for load would work like this:
If the record has a primary key(s) defined and the
object has this set, the object will be loaded (or reloaded)
from the DB.
If there is no primary key or one is not set, it will
attempt to find a single record that matches all of the
"set" fields. If there is only one match, it load the
object. If there is more than one, it throws an exception.
This would be similar to the current Peer methods that
take an record object and build criteria from it.
The nice thing about this is that if you have tables
with other unique attributes, e.g. say a users table with
a numeric PK, but the userId is also unique. You can
do code like:
User u = new User();
u.setUserId("foo");
u.setPassword("secret");
u.load();
or
User u = new User();
u.setId(123);
u.load();
or
User u = new User();
u.setEmail("foo@bar.org");
u.load();
Depending on your needs. And if you have an existing
object and need to ensure the values match the database
one, just do:
rec.load();
FWIW, I started with the assumption about tracking field
level state because of the problems that can occure if
there are default values. IMHO, default values make the
current way to find things using the record object methods
very unreliable. The defaults get added into the Criteria
that is built and so you may not get what you expected
using these methods.
Thus the need for tracking "set" values vs defaults. Even
with true support for determining NULL values is not enough
because the field value you want to search for might be NULL.
E.g., if I have a table with 10 fields that have default
values and allow NULLS, and do something like:
RecordOm rec = new RecordOm();
rec.setField1(value1);
rec.setField2(value2);
rec.setField3(null);
List results = RecordOmPeer.doSelect(rec);
I should get records with field1=value1, field2=value2, and
field3=NULL and no more.
A way to do this would be to only allow the field storage
objects be accessed via "set" methods. Preferrably a single
set method for each fields (or a common setByName.... type
method). I.e. this.privateField= ... should be discouraged
internally in the generated objects. We should always use
the setField(..) methods unless absolutely needed.
This way, anytime a value is set, a field state flag could
be set as well. This would allow for a method like:
isModified( columnName );
Note that default values would be set by object constructors
and not set the field state. This way a record object can be
examined to determine what fields need to be used in creating
a matching Criteria.
> -----Original Message-----
> From: Thomas Fischer [mailto:tfischer@apache.org]
> Sent: Thursday, December 14, 2006 10:10 AM
> To: Apache Torque Developers List
> Subject: Re: Torque 4.0 - Add delete() / loadByPk(...)
> methods to Record object
>
> +1 on both. Would it make sense to rename the loadByPk()
> method reload() ?
> If yes, it could make sense to generate the method also if no
> pk exists but then throwing an exception.
>
> I also like Thoams' idea of sessing isNew to triue after the
> object has been deleted.
>
> Do you mind adding this to the wiki page ?
> http://wiki.apache.org/db-torque/NextRelease
>
> Thomas
>
> On Tue, 5 Dec 2006, Greg Monroe wrote:
>
> > I've often thought it would be nice for the generated
> record object to
> > have a delete() and loadByPk() methods. It would help
> simplify a lot
> > of code.
> >
> > Of course the loadByPK method would only be added if there is a
> > primary key. And if there are multiple primary keys, the
> method sig.
> > would be something like:
> >
> > loadByPk( <colType> key1, <colType> key2...)
> > throw NoRowsException, TooManyRowsException
> >
> > and the delete() method would throw an exception if called on a new
> > record.
> > Greg Monroe <Mo...@DukeCE.com> (919)680-5050 C&IS
> Solutions Team Lead
> > Duke Corporate Education, Inc.
> > 333 Liggett St.
> > Durham, NC 27701
> >
> >
> >
> >
> > Duke CE 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: Torque 4.0 - Add delete() / loadByPk(...) methods to Record
object
Posted by Thomas Fischer <tf...@apache.org>.
+1 on both. Would it make sense to rename the loadByPk() method reload() ?
If yes, it could make sense to generate the method also if no pk exists
but then throwing an exception.
I also like Thoams' idea of sessing isNew to triue after the object has
been deleted.
Do you mind adding this to the wiki page ?
http://wiki.apache.org/db-torque/NextRelease
Thomas
On Tue, 5 Dec 2006, Greg Monroe wrote:
> I've often thought it would be nice for the
> generated record object to have a delete()
> and loadByPk() methods. It would help
> simplify a lot of code.
>
> Of course the loadByPK method would only be
> added if there is a primary key. And if there
> are multiple primary keys, the method sig. would
> be something like:
>
> loadByPk( <colType> key1, <colType> key2...)
> throw NoRowsException, TooManyRowsException
>
> and the delete() method would throw an exception
> if called on a new record.
> Greg Monroe <Mo...@DukeCE.com> (919)680-5050
> C&IS Solutions Team Lead
> Duke Corporate Education, Inc.
> 333 Liggett St.
> Durham, NC 27701
>
>
>
>
> Duke CE 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