Storage of Boolean in GenericEntity

[Adam Heath-2]

Why does GenericEntity store booleans internally as "Y" or "N"?
Wouldn't it make more sense to store them as Boolean?  "Y" or "N"
encoding should be handled at the database layer, when reading and writing.

[Jacques Le Roux]

I suppose it's related to HTML checkboxes (null or true) but this is more a guessing...

Jacques

De : "Adam Heath" <[hidden email]>


> Why does GenericEntity store booleans internally as "Y" or "N"?
> Wouldn't it make more sense to store them as Boolean?  "Y" or "N"
> encoding should be handled at the database layer, when reading and writing.
>

[David E Jones]

The Entity Engine rather intentionally does not try to hide things  
like this. Generally people working on applications have access to  
and work with:

1. seed/setup data files
2. the database itself (for debugging, integrations, and so on)
3. the code dealing with the field (writing, querying, comparing, etc)

If we just change #3 that would leave the other 2 inconsistent and  
confusing. Also in addition to #1 and #2 there are probably other  
things I'm missing in my quick thoughts on this.

-David


On Oct 15, 2007, at 9:29 AM, Adam Heath wrote:

> Why does GenericEntity store booleans internally as "Y" or "N"?
> Wouldn't it make more sense to store them as Boolean?  "Y" or "N"
> encoding should be handled at the database layer, when reading and  
> writing.

[Adam Heath-2]

David E Jones wrote:

>
> The Entity Engine rather intentionally does not try to hide things like
> this. Generally people working on applications have access to and work
> with:
>
> 1. seed/setup data files
> 2. the database itself (for debugging, integrations, and so on)
> 3. the code dealing with the field (writing, querying, comparing, etc)
>
> If we just change #3 that would leave the other 2 inconsistent and
> confusing. Also in addition to #1 and #2 there are probably other things
> I'm missing in my quick thoughts on this.

Anything in java code should deal with java values.  We don't deal with
strings when dealing with ints, floats, BigDecimal.  Why should we deal
with strings for booleans?

The entityengine is supposed to hide all database-specific code from the
application; but storing the Y/N for boolean in the java maps does *not*
accomplish that goal.

I never suggested just changing how GenericEntity deals with booleans.
Obviously all other parts of the code would need to be updated to deal
with booleans natively.

[Adam Heath-2]

Jacques Le Roux wrote:
> I suppose it's related to HTML checkboxes (null or true) but this is more a guessing...

Checkboxes are done with a isSet or empty, not a Y/N.  And radio is done
as a choice of possible values.

HTML forms(and parameters) are all sent as strings, and converted to a
java representation.  JDBC should be handled the same way.

If both of these external systems had their own conversion layers, and
gave the java code the same set of value objects to deal with, then
application programmers would only need to be concerned with dealing
with java objects.

[Adrian Crum]

I'm kinda with Adam on this one. The purpose of encapsulation is to hide implementation details. As
long as it doesn't involve a major change to the class's API, I can't see any harm in storing
internal data members in their native type.

-Adrian

Adam Heath wrote:

> David E Jones wrote:
>
>>The Entity Engine rather intentionally does not try to hide things like
>>this. Generally people working on applications have access to and work
>>with:
>>
>>1. seed/setup data files
>>2. the database itself (for debugging, integrations, and so on)
>>3. the code dealing with the field (writing, querying, comparing, etc)
>>
>>If we just change #3 that would leave the other 2 inconsistent and
>>confusing. Also in addition to #1 and #2 there are probably other things
>>I'm missing in my quick thoughts on this.
>
>
> Anything in java code should deal with java values.  We don't deal with
> strings when dealing with ints, floats, BigDecimal.  Why should we deal
> with strings for booleans?
>
> The entityengine is supposed to hide all database-specific code from the
> application; but storing the Y/N for boolean in the java maps does *not*
> accomplish that goal.
>
> I never suggested just changing how GenericEntity deals with booleans.
> Obviously all other parts of the code would need to be updated to deal
> with booleans natively.
>
>

[David E Jones]

On Oct 15, 2007, at 1:52 PM, Adam Heath wrote:

> David E Jones wrote:
>>
>> The Entity Engine rather intentionally does not try to hide things  
>> like
>> this. Generally people working on applications have access to and  
>> work
>> with:
>>
>> 1. seed/setup data files
>> 2. the database itself (for debugging, integrations, and so on)
>> 3. the code dealing with the field (writing, querying, comparing,  
>> etc)
>>
>> If we just change #3 that would leave the other 2 inconsistent and
>> confusing. Also in addition to #1 and #2 there are probably other  
>> things
>> I'm missing in my quick thoughts on this.
>
> Anything in java code should deal with java values.  We don't deal  
> with
> strings when dealing with ints, floats, BigDecimal.  Why should we  
> deal
> with strings for booleans?

Because they aren't booleans. There is no good standard for boolean  
data types in SQL.

> The entityengine is supposed to hide all database-specific code  
> from the
> application; but storing the Y/N for boolean in the java maps does  
> *not*
> accomplish that goal.

That is NOT the goal of the Entity Engine. It does NOT hide the  
database, it makes it easier to work with the database, and that's all.

Hiding the database is a BAD idea and makes various things harder,  
like understanding application data structures, optimizing queries  
and understanding why queries are slow, keeping structures free of  
redundancy, and so on.

> I never suggested just changing how GenericEntity deals with booleans.
> Obviously all other parts of the code would need to be updated to deal
> with booleans natively.

You could maybe do something about #1 to convert true/false to Y/N,  
but you really can't do anything about #2...

My opinion is this will make things more difficult, not easer. It  
will obfuscate and confuse, not open up and make clear.

-David

[Adam Heath-2]

David E Jones wrote:

>
> On Oct 15, 2007, at 1:52 PM, Adam Heath wrote:
>
>> David E Jones wrote:
>>>
>>> The Entity Engine rather intentionally does not try to hide things like
>>> this. Generally people working on applications have access to and work
>>> with:
>>>
>>> 1. seed/setup data files
>>> 2. the database itself (for debugging, integrations, and so on)
>>> 3. the code dealing with the field (writing, querying, comparing, etc)
>>>
>>> If we just change #3 that would leave the other 2 inconsistent and
>>> confusing. Also in addition to #1 and #2 there are probably other things
>>> I'm missing in my quick thoughts on this.
>>
>> Anything in java code should deal with java values.  We don't deal with
>> strings when dealing with ints, floats, BigDecimal.  Why should we deal
>> with strings for booleans?
>
> Because they aren't booleans. There is no good standard for boolean data
> types in SQL.

The entity engine allows java code to deal with booleans, and *does*
give a cross-database encoding for booleans.  Just because these fields
are not stored as booleans in the database, doesn't mean they shouldn't
be treated as booleans in the application code.

>> The entityengine is supposed to hide all database-specific code from the
>> application; but storing the Y/N for boolean in the java maps does *not*
>> accomplish that goal.
>
> That is NOT the goal of the Entity Engine. It does NOT hide the
> database, it makes it easier to work with the database, and that's all.

And dealing with booleans in application code *is* easier than dealing
with "Y"/"N".

Are you saying strings are easier than booleans?  Is that what you are
*really* saying?

> Hiding the database is a BAD idea and makes various things harder, like
> understanding application data structures, optimizing queries and
> understanding why queries are slow, keeping structures free of
> redundancy, and so on.

There is redundant code all over dealing with "Y"/"N", instead of
boolean values.  Think about scripting code that can automatically
handle boxed booleans natively; using "Y"/"N" does not allow such
unboxing to take place.

>> I never suggested just changing how GenericEntity deals with booleans.
>> Obviously all other parts of the code would need to be updated to deal
>> with booleans natively.
>
> You could maybe do something about #1 to convert true/false to Y/N, but
> you really can't do anything about #2...

The database will store it as Y/N, handling during read/write from jdbc.
 HTTP parameters can be encoded as Y/N as well.  And seed xml files.
But the in-memory specification should be native java objects.

All other field types are native objects;
clob/blob/timestamp/date/time/float/double/integer/long/BigDecimal.
Only boolean is special.  Remove the special casing.

> My opinion is this will make things more difficult, not easer. It will
> obfuscate and confuse, not open up and make clear.

Well, I think your wrong there.  Seeing a "Y"/"N" in the code makes me
questioning if the field is a single character, holding "Y"/"N"/"A"/"1",
or something else.  A boolean type is obvious.

[David E Jones]

Adam,

It's pretty clear you like this idea and don't want to back down, so  
I'm not going to push back.

What is wrong with the code already in the GenericEntity.java file  
that does this boolean conversion?

As far as making it a best practice goes... maybe you should read  
through the best practices list and see if you can find where it  
might for it. For form, screen, tree, and menu widgets and for simple-
methods it's a non-issue as Y/N are actually easier and less  
confusing (zero mapping beats single mapping any day). This only has  
an effect for business logic written Java, and I think that's a bad  
idea in the first place.

So:

1. small issue; not going to save anyone any significant amount of time
2. bad for the best practices tools

If you want to argue with the best practices... feel free to start up  
a thread, just try to keep in mind limited resources, better things  
to work on for EVERYONE, and general ROI (starting with proving any  
sort of return, before even thinking about the investment required).

http://docs.ofbiz.org/display/OFBADMIN/Best+Practices+Guide

-David


On Oct 15, 2007, at 3:50 PM, Adam Heath wrote:

> David E Jones wrote:
>>
>> On Oct 15, 2007, at 1:52 PM, Adam Heath wrote:
>>
>>> David E Jones wrote:
>>>>
>>>> The Entity Engine rather intentionally does not try to hide  
>>>> things like
>>>> this. Generally people working on applications have access to  
>>>> and work
>>>> with:
>>>>
>>>> 1. seed/setup data files
>>>> 2. the database itself (for debugging, integrations, and so on)
>>>> 3. the code dealing with the field (writing, querying,  
>>>> comparing, etc)
>>>>
>>>> If we just change #3 that would leave the other 2 inconsistent and
>>>> confusing. Also in addition to #1 and #2 there are probably  
>>>> other things
>>>> I'm missing in my quick thoughts on this.
>>>
>>> Anything in java code should deal with java values.  We don't  
>>> deal with
>>> strings when dealing with ints, floats, BigDecimal.  Why should  
>>> we deal
>>> with strings for booleans?
>>
>> Because they aren't booleans. There is no good standard for  
>> boolean data
>> types in SQL.
>
> The entity engine allows java code to deal with booleans, and *does*
> give a cross-database encoding for booleans.  Just because these  
> fields
> are not stored as booleans in the database, doesn't mean they  
> shouldn't
> be treated as booleans in the application code.
>
>>> The entityengine is supposed to hide all database-specific code  
>>> from the
>>> application; but storing the Y/N for boolean in the java maps  
>>> does *not*
>>> accomplish that goal.
>>
>> That is NOT the goal of the Entity Engine. It does NOT hide the
>> database, it makes it easier to work with the database, and that's  
>> all.
>
> And dealing with booleans in application code *is* easier than dealing
> with "Y"/"N".
>
> Are you saying strings are easier than booleans?  Is that what you are
> *really* saying?
>
>> Hiding the database is a BAD idea and makes various things harder,  
>> like
>> understanding application data structures, optimizing queries and
>> understanding why queries are slow, keeping structures free of
>> redundancy, and so on.
>
> There is redundant code all over dealing with "Y"/"N", instead of
> boolean values.  Think about scripting code that can automatically
> handle boxed booleans natively; using "Y"/"N" does not allow such
> unboxing to take place.
>
>>> I never suggested just changing how GenericEntity deals with  
>>> booleans.
>>> Obviously all other parts of the code would need to be updated to  
>>> deal
>>> with booleans natively.
>>
>> You could maybe do something about #1 to convert true/false to Y/
>> N, but
>> you really can't do anything about #2...
>
> The database will store it as Y/N, handling during read/write from  
> jdbc.
>  HTTP parameters can be encoded as Y/N as well.  And seed xml files.
> But the in-memory specification should be native java objects.
>
> All other field types are native objects;
> clob/blob/timestamp/date/time/float/double/integer/long/BigDecimal.
> Only boolean is special.  Remove the special casing.
>
>> My opinion is this will make things more difficult, not easer. It  
>> will
>> obfuscate and confuse, not open up and make clear.
>
> Well, I think your wrong there.  Seeing a "Y"/"N" in the code makes me
> questioning if the field is a single character, holding  
> "Y"/"N"/"A"/"1",
> or something else.  A boolean type is obvious.
>

[Adam Heath-2]

David E Jones wrote:
>
> Adam,
>
> It's pretty clear you like this idea and don't want to back down, so I'm
> not going to push back.

It's been a problem for me(and Ean, my coworker) since we started using
OfBiz.  It's not something new.  It's made our job *more* difficult.

> What is wrong with the code already in the GenericEntity.java file that
> does this boolean conversion?

> As far as making it a best practice goes... maybe you should read
> through the best practices list and see if you can find where it might
> for it. For form, screen, tree, and menu widgets and for simple-methods
> it's a non-issue as Y/N are actually easier and less confusing (zero
> mapping beats single mapping any day). This only has an effect for
> business logic written Java, and I think that's a bad idea in the first
> place.

What do you mean, by zero-mapping vs single-mapping?

When I see code that compares a value to Y/N, I assume it's a
single-character string field, and can store any single character.  A
boolean comparison, on the other hand, tells me exactly what the field
holds.

> So:
>
> 1. small issue; not going to save anyone any significant amount of time

For those that have dealt with OfBiz from the beginning, you are right,
that this will hurt their knowledge of the framework.

However, new users coming into the system have no idea about what the
Y/N stuff is about; they deal with java objects, and those are booleans.

We *want* new people to use OfBiz.  And this special case will cause
them grief.

> 2. bad for the best practices tools
>
> If you want to argue with the best practices... feel free to start up a
> thread, just try to keep in mind limited resources, better things to
> work on for EVERYONE, and general ROI (starting with proving any sort of
> return, before even thinking about the investment required).

Switching to boolean values for boolean fields *will* reduce the code on
the system.

Another point; in lots of simple method definitions, there are blocks
for doing CRUD to entities; in the vast majority of cases, these method
definitions are *identical*, except for the entity name and property to
pull error messages from.  This points to a need to refactor.

> http://docs.ofbiz.org/display/OFBADMIN/Best+Practices+Guide

Nothing about booleans there.  And it mentions CVS, which is years out
of date.

And what is OOTB?  It's used, but never defined.

[David E Jones]

On Oct 15, 2007, at 4:48 PM, Adam Heath wrote:

> David E Jones wrote:
>>
>> Adam,
>>
>> It's pretty clear you like this idea and don't want to back down,  
>> so I'm
>> not going to push back.
>
> It's been a problem for me(and Ean, my coworker) since we started  
> using
> OfBiz.  It's not something new.  It's made our job *more* difficult.

Could you be more specific? What has made your job more difficult?

-David

[Adam Heath-2]

David E Jones wrote:

>
> On Oct 15, 2007, at 4:48 PM, Adam Heath wrote:
>
>> David E Jones wrote:
>>>
>>> Adam,
>>>
>>> It's pretty clear you like this idea and don't want to back down, so I'm
>>> not going to push back.
>>
>> It's been a problem for me(and Ean, my coworker) since we started using
>> OfBiz.  It's not something new.  It's made our job *more* difficult.
>
> Could you be more specific? What has made your job more difficult?

MyEvent.groovy:

== What I currently do:

def value = delegator.findByPrimaryKeyCache("EntityName", [fieldName:
"fieldValue"])
if ("Y".equals(value.booleanField)) {
        // default false logic
==

== What I'd like to do:
if (value.booleanField) {
        // default false logic
==

Groovy treats maps as objects with properties, and can automatically
pull out fields.  It also does automatic boxing/unboxing, including
handling nulls(so that sql trinary logic works as expected).

Additionally, Boolean.TRUE.equals(value.get("booleanField")) is easier
to understand, than "Y".equals(value.get("booleanField")).  In the
latter case, which scanning the code, it's hard to know if booleanField
is really a boolean-typed field, or a single-character string field.
I'd have to look at the entity definition to be sure.

So, there's a valid concern.  Seeing a compare to a "Y" or an "N", could
mean it's a single-character field, a string field, *or* a boolean
field.  It's not known by looking at just the code in question.  Whereas
if it was a boolean compare, there would be no doubt.

Additionally, as mentioned before, new users seeing ofbiz for the first
time are used to dealing with their language's native objects.  This Y/N
pattern is not really a boolean, but an attempt to graft something new
into the programming system.  It's more to learn.

[David E Jones]

On Oct 15, 2007, at 5:21 PM, Adam Heath wrote:

> David E Jones wrote:
>>
>> On Oct 15, 2007, at 4:48 PM, Adam Heath wrote:
>>
>>> David E Jones wrote:
>>>>
>>>> Adam,
>>>>
>>>> It's pretty clear you like this idea and don't want to back  
>>>> down, so I'm
>>>> not going to push back.
>>>
>>> It's been a problem for me(and Ean, my coworker) since we started  
>>> using
>>> OfBiz.  It's not something new.  It's made our job *more* difficult.
>>
>> Could you be more specific? What has made your job more difficult?
>
> MyEvent.groovy:
>
> == What I currently do:
>
> def value = delegator.findByPrimaryKeyCache("EntityName", [fieldName:
> "fieldValue"])
> if ("Y".equals(value.booleanField)) {
> // default false logic
> ==
>
> == What I'd like to do:
> if (value.booleanField) {
> // default false logic
> ==

So what is making your job more difficult is having to do this:

if ("Y".equals(value.isSomething))

instead of this:

if (value.isSomething)

Is that right?

If you're looking for easy to understand this has been supported for  
half a dozen years:

if (value.getBoolean(isSomething).booleanValue())

Or if Groovy is like bsh you can just do:

if (value.getBoolean(isSomething))

Is that what you are looking for? If so I apologize as I was  
confused. I thought you were proposing that we force this throughout.  
The option to treat the Y/N values as booleans has been around for a  
long time.

If you're proposing that we change the get method to behave like the  
getBoolean method by default, then I'd be pretty against that as it  
would break a LOT code/etc, much of which is not compiled.

Whatever the case, again please be more specific about what you  
propose... I kind of feel like I'm spinning my wheels... ;)

-David

[Adam Heath-2]

David E Jones wrote:

> Is that what you are looking for? If so I apologize as I was confused. I
> thought you were proposing that we force this throughout. The option to
> treat the Y/N values as booleans has been around for a long time.

Wasn't looking for anything.  I am aware of the methods on GenericEntity.

> If you're proposing that we change the get method to behave like the
> getBoolean method by default, then I'd be pretty against that as it
> would break a LOT code/etc, much of which is not compiled.

I am proposing that, but it might wait for an extended period.  Code
coverage will help with it.  And my grep foo is rather good and finding
these things.

I'm willing to do this in a git/svk branch, if there is worry about
breakage.

> Whatever the case, again please be more specific about what you
> propose... I kind of feel like I'm spinning my wheels... ;)

Ok, let's do this another way:

If I want an Integer from an entity, I do:

        (Integer) value.get("integerField");

If I want a Timestamp from an entity, I do:

        (java.sql.Timestamp) value.get("timestampField");

If I want a BigDecimal from an entity, I do:

        (BigDecimal) value.get("bigDecimalField");

If I want a Blob from an entity, I do:

        (Blob) value.get("blobField");

If I want a Boolean from an entity, I do:

        value.getBoolean("booleanField");

or:
        "Y".equals(value.get("booleanField"))

Note the pattern.

[Ludovic Maître]

Hi all,

Although i doesn't contribute to the codebase yet, i would like to say
that i totally agree with Adam. This is stupid to do checks on character
where we can do simple checks on boolean. I didn't see the point in
arguing all the night that it's better to do string comparison than
boolean comparison. (and i'm not a beginner at programming)

Best regards,

Adam Heath a écrit :

> David E Jones wrote:
>
>  
>> Is that what you are looking for? If so I apologize as I was confused. I
>> thought you were proposing that we force this throughout. The option to
>> treat the Y/N values as booleans has been around for a long time.
>>    
>
> Wasn't looking for anything.  I am aware of the methods on GenericEntity.
>
>  
>> If you're proposing that we change the get method to behave like the
>> getBoolean method by default, then I'd be pretty against that as it
>> would break a LOT code/etc, much of which is not compiled.
>>    
>
> I am proposing that, but it might wait for an extended period.  Code
> coverage will help with it.  And my grep foo is rather good and finding
> these things.
>
> I'm willing to do this in a git/svk branch, if there is worry about
> breakage.
>
>  
>> Whatever the case, again please be more specific about what you
>> propose... I kind of feel like I'm spinning my wheels... ;)
>>    
>
> Ok, let's do this another way:
>
> If I want an Integer from an entity, I do:
>
> (Integer) value.get("integerField");
>
> If I want a Timestamp from an entity, I do:
>
> (java.sql.Timestamp) value.get("timestampField");
>
> If I want a BigDecimal from an entity, I do:
>
> (BigDecimal) value.get("bigDecimalField");
>
> If I want a Blob from an entity, I do:
>
> (Blob) value.get("blobField");
>
> If I want a Boolean from an entity, I do:
>
> value.getBoolean("booleanField");
>
> or:
> "Y".equals(value.get("booleanField"))
>
> Note the pattern.
>
>
>  

--
Cordialement,
Ludo - http://www.ubik-products.com
---
"L'amour pour principe et l'ordre pour base; le progres pour but" (A.Comte)

[Jacques Le Roux]

De : "Adam Heath" <[hidden email]>

> Another point; in lots of simple method definitions, there are blocks
> for doing CRUD to entities; in the vast majority of cases, these method
> definitions are *identical*, except for the entity name and property to
> pull error messages from.  This points to a need to refactor.

IMHO, this point is very interesting, but of course need some work...

Jacques

[Adrian Crum]

Something similar was done with permission checking - see the CommonPermissionServices.xml file in
framework/common and how it is used in FixedAssetMaintServices.xml in specialpurpose/assetmaint.


Jacques Le Roux wrote:

> De : "Adam Heath" <[hidden email]>
>
>>Another point; in lots of simple method definitions, there are blocks
>>for doing CRUD to entities; in the vast majority of cases, these method
>>definitions are *identical*, except for the entity name and property to
>>pull error messages from.  This points to a need to refactor.
>
>
> IMHO, this point is very interesting, but of course need some work...
>
> Jacques
>

[David E Jones]

On Oct 16, 2007, at 12:51 PM, Jacques Le Roux wrote:

> De : "Adam Heath" <[hidden email]>
>
>> Another point; in lots of simple method definitions, there are blocks
>> for doing CRUD to entities; in the vast majority of cases, these  
>> method
>> definitions are *identical*, except for the entity name and  
>> property to
>> pull error messages from.  This points to a need to refactor.
>
> IMHO, this point is very interesting, but of course need some work...

These were designed VERY specifically to be this way. It is true  
there are only a few common variations for different data structures  
and corresponding create/updated/delete operations.

The point of having simple-methods exist for this instead of just  
have "call create variation A" in the service definition is to make  
it easy to customize and enhance. As functionality around certain  
entities grows these methods do tend to grow beyond the base 2-4  
operations that the typical minimal methods include.

So, yeah, it was intentional to have separate operations for things  
like (all of these are just for creates, different combinations for  
different things):

1. make a value object
2. put a sequenced id in a single pk field for the object
3. move all primary key fields from the parameters or other Map into  
the value
4. move non-pk fields from a Map into the value
5. set individual fields on the value
6. and so on...

We could identify a few variations and have single operations, or  
avoid writing simple-methods altogether with the service def thing I  
mentioned above, but that would just reduce consistency between  
different simple-methods and make customization and extension more  
difficult.

It seems like some recent comments stem from a disagreement that  
consistency and ease of customization are important... that might be  
an interesting thing to talk about explicitly. Much of the OFBiz  
framework is meant for help just those things, where necessary  
sacrificing other priorities, all part of optimizing the development  
experience at the XML file level.

The driving factor behind that priority is that in most large  
software it is complexity that kills projects and causes budgets to  
spiral out of control. As complexity increases the time and cost  
tends to increase in a non-linear way, ie something like  
exponentially or logarithmically instead of linearly. When I say  
complexity I mean general solution complexity across hundreds of  
entities and services and screens, and not the complexity of a single  
service. The point of defining the scope and purpose of a service is  
to limit the complexity of that one piece to make it possible to  
write and maintain it with a reasonable volume of requirements.

For OFBiz (both the framework and the applications) it is critical  
that we all keep this in mind. It does us no good to create something  
that supports super fast development if it makes ongoing maintenance  
and extension/customization more difficult (including other people's  
code or years later when you've forgotten and it might as well be  
something someone else wrote). The point is to be as clear and  
consistent as possible across a variety of data structures and  
business processes.

When talking about framework improvements that should be the first  
thing considered and written about when presenting ideas. Ideas that  
don't help with that will be fought pretty hard by various people, me  
being the most vocal, but rest assured that if I shut up others will  
fill in because lots of people in the ofbiz community have experience  
with this and know what kind of a difference it makes.

-David

[Adam Heath-2]

David E Jones wrote:
> These were designed VERY specifically to be this way. It is true there
> are only a few common variations for different data structures and
> corresponding create/updated/delete operations.
>
> The point of having simple-methods exist for this instead of just have
> "call create variation A" in the service definition is to make it easy
> to customize and enhance. As functionality around certain entities grows
> these methods do tend to grow beyond the base 2-4 operations that the
> typical minimal methods include.

The refactoring I mentioned never talked about implementation of the
common services; they could very well still be simple methods.

> So, yeah, it was intentional to have separate operations for things like
> (all of these are just for creates, different combinations for different
> things):
>
> 1. make a value object
> 2. put a sequenced id in a single pk field for the object
> 3. move all primary key fields from the parameters or other Map into the
> value
> 4. move non-pk fields from a Map into the value
> 5. set individual fields on the value
> 6. and so on...
>
> We could identify a few variations and have single operations, or avoid
> writing simple-methods altogether with the service def thing I mentioned
> above, but that would just reduce consistency between different
> simple-methods and make customization and extension more difficult.
>
> It seems like some recent comments stem from a disagreement that
> consistency and ease of customization are important... that might be an
> interesting thing to talk about explicitly. Much of the OFBiz framework
> is meant for help just those things, where necessary sacrificing other
> priorities, all part of optimizing the development experience at the XML
> file level.
>
> The driving factor behind that priority is that in most large software
> it is complexity that kills projects and causes budgets to spiral out of
> control. As complexity increases the time and cost tends to increase in
> a non-linear way, ie something like exponentially or logarithmically
> instead of linearly. When I say complexity I mean general solution
> complexity across hundreds of entities and services and screens, and not
> the complexity of a single service. The point of defining the scope and
> purpose of a service is to limit the complexity of that one piece to
> make it possible to write and maintain it with a reasonable volume of
> requirements.

Exactly the point I'm trying to make.

If every CRUD implementation on every entity has it's own
implementation, then there are that many *more* things that have to be
learned.

However, if you see that entity #1 uses one implementation, then entity
#2 uses it, and entity #3 uses it, ..., then that's *less* that you have
to learn.

It should *always* be possible to have per-entity implementations; no
one is suggesting that that feature be removed.  But reducing code, by
refactoring, and reducing the learning requirements, *is* a win.

Commonality is *good*.  If it weren't, we wouldn't have an entity
engine, or a service engine.  We'd have hard-coded jdbc calls and bsf
usage scattered all over.

If just 2 entities share the same CRUD implementation, then we have
reduced the knowledge workload, and that is a win.

> For OFBiz (both the framework and the applications) it is critical that
> we all keep this in mind. It does us no good to create something that
> supports super fast development if it makes ongoing maintenance and
> extension/customization more difficult (including other people's code or
> years later when you've forgotten and it might as well be something
> someone else wrote). The point is to be as clear and consistent as
> possible across a variety of data structures and business processes.

Please stop making my points for me.

[David E Jones]

On Oct 16, 2007, at 2:51 PM, Adam Heath wrote:

>> The driving factor behind that priority is that in most large  
>> software
>> it is complexity that kills projects and causes budgets to spiral  
>> out of
>> control. As complexity increases the time and cost tends to  
>> increase in
>> a non-linear way, ie something like exponentially or logarithmically
>> instead of linearly. When I say complexity I mean general solution
>> complexity across hundreds of entities and services and screens,  
>> and not
>> the complexity of a single service. The point of defining the  
>> scope and
>> purpose of a service is to limit the complexity of that one piece to
>> make it possible to write and maintain it with a reasonable volume of
>> requirements.
>
> Exactly the point I'm trying to make.
>
> If every CRUD implementation on every entity has it's own
> implementation, then there are that many *more* things that have to be
> learned.

Um, but that's NOT the point I was making. You're talking about  
sacrificing flexibility and ease of customization for ease of initial  
implementation. As for understanding and reading, once you've seen  
one of each pattern you've seen them all so there isn't anything more  
to keep a handle on, until you find an exception to the pattern you  
are used to and then it's important to look and understand why it is  
different, and yada yada yada.

-David