In my opinion, one thing to consider is the monumental amount of code out
there that depends on the Ofbiz core.  Ofbiz is not generally usable ootb.
It requires considerable modification and additions for industry specific
needs.  I myself have 321 java files containing 6+ million bytes of code and
maybe as many bsh scripts that make calls to the entity engine and that is
for 1 of 4 installations I have done.I am sure that I am only one of many.

As for findAll(), it makes good sense if someone wants to deprecate it.
After a while, you get annoyed at the deprecation warnings and you fix your
code to use the recommended version.  It makes almost no sense to remove it
altogether unless the underlying logic has also been removed because of a
conflict.  It amounts to about 10 bytes in the jar file calling another
function with null arguments.

Also, findAll does what it says.  findList(...) requires a comment for
others less familiar with the entity engine who maintain this code.  We have
only one developer with Ofbiz skills.  We have many more with great java
skills that work on the same code base.  It makes sense to me to make it as
easy to read and understand especially considering the dearth of comments in
the javadoc.

I agree that it is a moot point.  I have added all these removed functions
(the few I use) back in and my jar file increased 98 bytes.  Problem solved.

BTW, getting into more and more of this and it was a great job you guys have
done with the lookup screens automatically generating JSON for
auto-completion.

Skip

-----Original Message-----
From: Jacques Le Roux [mailto:[email protected]]
Sent: Thursday, May 30, 2013 12:33 PM
To: [email protected]
Subject: Re: But in CategoryTree.groovy


About findAll(), yes it's a moot point

David got into this direction 5 years ago and nobody was really against it
http://markmail.org/message/thcsl7khketkqagh

One of the ideas was to get rif of all the variations and use rather
parameters, take for instance
http://svn.apache.org/viewvc?view=revision&revision=938948

IMO, Jonathan had a good point about owen buttons in the thread above
One thing we would consider is reintroducing most simplifed ones, like
findAll(), yes!

But maybe Scott would be agains it
http://svn.apache.org/viewvc?view=revision&revision=938947

Opinions?

Jacques

From: "Skip" <[email protected]>
> Jacques
>
> That works fine.  findAll() works for me because I added it back in.
> findAll() is a better function to use because in my opinion, it sez
exactly
> what it does and that makes the code more readable.  I have hundreds of
java
> files written over the years and it was way easier to add back in the
> functionality you guys deprecated/removed than to replace all my entity
> engine calls.
>
> This time however, I put them all together so it would be easier next time
> you guys do a major release.
>
> BTW, I have had a few weeks now to go pretty thoroughly through lots of
this
> code and it is very nice.  I am especially pleased with the re-write of
the
> widget code and the use of jquery.
>
> Skip
>
> -----Original Message-----
> From: Jacques Le Roux [mailto:[email protected]]
> Sent: Thursday, May 30, 2013 6:38 AM
> To: [email protected]
> Subject: Re: But in CategoryTree.groovy
>
>
> Thanks for report Skip,
>
> BTW you must have missed something, findAll does not exist anymore for 3
> years ;)
> http://svn.apache.org/viewvc?view=revision&revision=938947
>
> Using
> prodCatalogs = delegator.findList("ProdCatalog", null, null, null, null,
> false);
> Should be the way. Could you please confirm before I commit?
>
> Thanks
>
> Jacques
>
> From: "Skip" <[email protected]>
>> There is a line at 50 that sez:
>>
>> prodCatalogs = delegator.findByAnd("ProdCatalog");
>>
>> This causes a groovy ambiguious error when you go to the catalog screen
in
>> 12.04.
>>
>> Changing this to:
>> prodCatalogs = delegator.findAll("ProdCatalog");
>>
>> Fixes the problem.
>>
>> I expect you guys already did this, there here it is for any others like
> me
>> that just download the zip file.
>>
>> Skip
>>
>

Reply via email to