HotWax Media http://www.hotwaxmedia.com
On 10/12/2010, at 8:58 AM, Scott Gray wrote:
> On 10/12/2010, at 8:54 AM, Adam Heath wrote:
>
>> On 12/09/2010 01:48 PM, Scott Gray wrote:
>>> 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)?
>>
>> public final class EntityBuilderUtil {
>> public static EntityOne one(Delegator delegator) {
>> return new EntityOne(delegator);
>> }
>>
>> public static EntityList list(Delegator delegator) {
>> return new EntityList(delegator);
>> }
>> }
>>
>> This also means that java api code only needs to import one class. Plus, if
>> this class is used as a groovy category, then groovy code can do this:
>>
>> one(delegator).cache(true).....
>>
>> As groovy will see one as a method call, taking a variable with type
>> Delegator, and search all its categories to find a matching method. You'll
>> get all things for free.
>
> Works for me.
>
> Thanks
> Scott
Patch attached to https://issues.apache.org/jira/browse/OFBIZ-4053
Regards
Scott
smime.p7s
Description: S/MIME cryptographic signature
