On 10/12/2010, at 5:53 AM, Adam Heath wrote:
> On 12/09/2010 12:03 AM, Scott Gray wrote:
>> On 9/12/2010, at 11:50 AM, Adam Heath wrote:
>>
>>> On 12/08/2010 03:00 PM, Scott Gray wrote:
>>>> On 8/12/2010, at 4:29 AM, Adam Heath wrote:
>>>>
>>>>> I've deprecated findByAnd locally, replacing calls with a variant that
>>>>> takes a boolean, which specifies whether to use the cache.
>>>>>
>>>>> Initially, my replacement just added a new findByAnd. However, I'm
>>>>> now starting to lean towards replacing with findList instead.
>>>>> However, in my opinion, I think that will make programming ofbiz more
>>>>> difficult.
>>>>>
>>>>> I'd like to add a findList variant, that takes a Map instead of an
>>>>> EntityCondition. Without this new variant, there would be ton of
>>>>> variants that would only need to import EntityCondition, just to
>>>>> create a condition.
>>>>>
>>>>> There are also performance considerations to consider.
>>>>>
>>>>> EntityCondition.makeCondition(Map) directly takes the map, doing no
>>>>> processing. However, EntityCondition.makeCondition(Object...)
>>>>> eventually calls EntityUtil.makeFields, which does a little more than
>>>>> UtilMisc. In addition to the iteration over the array, it also does a
>>>>> check on the value for Comparable/Serializable. This latter check
>>>>> seems a bit superfluous, as eventually the base condition classes
>>>>> check the values against the model.
>>>>>
>>>>> So, from a purist stand point, even tho findByAnd could be replaced by
>>>>> findList, I think it is too useful; it saves enough code in
>>>>> application layers, imho.
>>>>>
>>>>
>>>> This reminded me of the query objects with method chaining that I
>>>> suggested a while back so I threw them together this morning.
>>>>
>>>> Here are some examples:
>>>> thisContent = delegator.findByPrimaryKeyCache("Content",
>>>> UtilMisc.toMap("contentId", assocContentId));
>>>> // becomes
>>>> thisContent =
>>>> EntityOne.query(delegator).from("Content").where("contentId",
>>>> assocContentId).cache().find();
>>>
>>> api:
>>> EntityOne(delegator).from()....
>>>
>>> in foo.groovy:
>>> use(DelegatorCategory) {
>>>
>>> }
>>>
>>> class DelegatorCategory {
>>> public static EntityOneBuilder EntityOne(Delegator delegator) {
>>> return new EntityOneBuilder(delegator);
>>> }
>>> }
>>> class EntityOneBuilder {
>>> public EntityOneBuilder from(String entityName) {
>>> return this;
>>> }
>>>
>>> public GenericValue query() {
>>> return delegator.findOne(....);
>>> }
>>> }
>>>
>>> This is almost like what you suggested, but it removes the query() method
>>> that starts thte builder, and changes the find() to query().
>>>
>>> EntityList would be done the same one.
>>>
>>> The way you have it, is that the start(EntityOne, EntityList) and the
>>> end(find(), list()), are 2 things that have to be remembered. My version
>>> just has one thing(the start of the call).
>>
>> This is all groovy DSL related though right? I hadn't worried about groovy
>> too much because I knew we had a fair bit of flexibility thanks to the
>> language features. The API I've written to date was solely intended for
>> java development but seems succinct enough that not much would need to
>> change for groovy.
>>
>> Also EntityList's execute methods so far are:
>> list()
>> iterator()
>> first()
>> count()
>
> All primary methods should be query(), imho.
>
> interface GenericQueryBuilder<T> {
> T query();
> }
>
> public class EntityOne implements GenericQueryBuilder<GenericValue> {
> public GenericValue query() {}
> }
>
> public class EntityList implements GenericQueryBuilder<List<GenericValue>>,
> Iterable<GenericValue> {
> public List<GenericValue> query() {}
> public Iterator<GenericValue> iterator() {}
> ...
> }I'm not opposed to that, but I'll need another method name for specifying the delegator. How about use(delegator)? Regards Scott
smime.p7s
Description: S/MIME cryptographic signature
