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).


List productNodesList = delegator.findByAndCache("ProductAssoc", 
UtilMisc.toMap("productIdTo", productId, "productAssocTypeId", bomType));
productNodesList = EntityUtil.filterByDate(productNodesList, inDate);
// becomes
List productNodesList = 
EntityList.query(delegator).from("ProductAssoc").where("productIdTo", productId, 
"productAssocTypeId", bomType).cache().filterByDate().list();

List<GenericValue>  inventoryItems = delegator.findByAnd("InventoryItem", ecl, 
null, null, null, false);
// becomes
List<GenericValue>  inventoryItems = 
EntityList.query(delegator).from("InventoryItem").where(ecl).list();

// all this (long winded example)
EntityFindOptions efo = new EntityFindOptions();
efo.setDistinct(true);
efo.setResultSetType(EntityFindOptions.TYPE_SCROLL_INSENSITIVE);
if (maxResults != null) {
     efo.setMaxRows(maxResults);
}
eli = delegator.findListIteratorByCondition(dynamicViewEntity, whereCondition, 
null, fieldsToSelect, orderByList, efo);
// becomes
eli = 
EntityList.query(delegator).from(dynamicViewEntity).where(whereCondition).select(fieldsToSelect).orderBy(orderByList).distinct().cursorScrollInsensitive().maxRows(maxResults).iterator();

They aren't necessarily shorter, but they are easier to write, easier to read 
and would be easier to remember (for things like groovy).

Regards
Scott

HotWax Media
http://www.hotwaxmedia.com


Reply via email to