Re: [PATCH TORQUE-3.1.1] Name Boolean/boolan getters correctly for

["Henning P. Schmiedehausen" <hp...@intermeta.de>]

Scott Eade <se...@backstagetech.com.au> writes:

>Henning,

>I like what this patch does, but I have a couple of concerns.  Firstly I 

[Was busy for a while, so I answer just now ]

>think the fact that this would result in Torque 3.1.1 generating 
>different method names to Torque 3.1 (at least for boolean columns) is 
>not something users would be expecting.  Secondly, from the perspective

True. I fell into a "gotcha" with another application [1], which uses
the regular bean classes for object parsing and that expected
"is<xxx>" methods for the binary columns. This is a Catch-22
situation. The current code generator does it wrong. However, users
are used to this and work around it. If we never change it, there will
be no improvement.
 
>of a Turbine user, have/will the mapper methods in ParameterParser, et 
>al, been/be updated?  I know that my Turbine applications would fail if 

Nope. I wasn't even thinking about this and I don't think that this is
an issue, mainly because the setProperties() method calls the
_setters_ and they don't change. 

Or do you map _from_ a bean to a ParameterParser (strange idea, I
never tried to do this).

>this patch is applied and I regenerate my om.  IMHO it would be more 
>appropriate to introduce such a change in Torque 3.2.  A more acceptable 
>alternative would be if the generated code provides both the current 
>"get" methods (perhaps marking them as deprecated) and the new "is 
>methods.  It would be nice to know that the Turbine ParameterParser was 

Don't do this! I tried it in my business classes and funnily enough,
there is software which chokes if it finds a boolean get<something>
method and a boolean is<something> method. I don't know whether this
is a regular behaviour or a plain bug but in my case, I had to work
around it.

>going to sync up with this change in some future release.

If the Turbine parser relies on getter and setter methods being called
get<xxx> and set<xxx>, it will be in for more nasty surprises. IIRC,
ParameterParser calls getWriteMethod() and getReadMethod() on a
PropertyDescriptor, so I see no problem here.


>A few additional minor points:
>1. I have highlighted a change below that I think you may like to 
>reverse (an erroneous replace I think).

Yes. That was a mistake. 

>2. I have also highlighted an instance where you introduce an space 
>character after method names in the generated code (I only highlighted 
>one of these but there are more).  While the generated code is not all 
>that attractive there is no reason to make it worse.  This was 
>highlighted to me by comparing changes to the generated code after 
>applying your patch - it results in a whole bunch of changes that are 
>just the introduction of a space.

Did I? It might be a search-and-replace mistake, adding a " " to the
replace string. No particular reason except sloppiness. :-)

>3. The Object.vm changes need to be replicated for ObjectWithManager.vm

Yes. However, I don't use this so I didn't want to break a class that
I don't use and don't know exactly how it works.

I agree with your analysis and I admit, that I feel a little uneasy
about dropping a method name change in the middle of a minor release.

However, I have the following points:

- Existing classes won't break. The code is generated at compile time,
  classes that have been generated won't break by this change. When recompiling
  source throws compilation errors, they can easily be fixed. There is no
  "binary replacement" problem like e.g. getList/getVector in commons-configuration
  or the commons-lang disaster.

- Properly written Software that uses the PropertyDescriptor and getGetterMethod
  and getSetterMethod won't break. 

- Some poorly written software, that chokes on boolean get<xxx> will work.

Disadvantage is, that business logic extending the BaseClasses which
might contain an boolean get<Foo> method to override the method in the
base class, must be adjusted. There is no way around it, yes.

I still want to lobby for this patch. If it won't make 3.1.1, we could
apply it immediately after the release and then use 3.1.2-dev to check
out if there are major user complaints.

	Regards
		Henning

[1] To name the culprit: commons-betwixt and <addDefaults /> in the
    XML description.
-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/

RedHat Certified Engineer -- Jakarta Turbine Development  -- hero for hire
   Linux, Java, perl, Solaris -- Consulting, Training, Development

"Fighting for one's political stand is an honorable action, but re-
 fusing to acknowledge that there might be weaknesses in one's
 position - in order to identify them so that they can be remedied -
 is a large enough problem with the Open Source movement that it
 deserves to be on this list of the top five problems."
                       -- Michelle Levesque, "Fundamental Issues with
                                    Open Source Software Development"

---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org