You are viewing a plain text version of this content. The canonical link for it is here.
Posted to torque-dev@db.apache.org by Henning Schmiedehausen <hp...@intermeta.de> on 2004/08/21 01:53:05 UTC
[PATCH TORQUE-3.1.1] Name Boolean/boolan getters correctly for
generated objects
Hi Scott,
I know that you are close to the 3.1.1 release of Torque but I would
really want you to consider the attached patch. It changes the
generation of the Getter and Setter methods in the Object.vm and Peer.vm
to ask the Column. The column object gets two additional getters, called
getGetterName() and getSetterName(). The getSetterName() just returns
"set$Property", but the getGetterName() returns "is$property" if the
property value is either boolean or a java.lang.Boolean.
This is in accordance to the Sun Java Beans rules and I would really
like to see this patch in the 3.1.1 release.
Regards
Henning
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
hps@intermeta.de +49 9131 50 654 0 http://www.intermeta.de/
Java, perl, Solaris, Linux, xSP Consulting, Web Services
RHCE - Consultant - Jakarta Turbine Development - hero for hire
"Fighting for one's political stand is an honourable 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"
Re: [PATCH TORQUE-3.1.1] Name Boolean/boolan getters correctly for
Posted by "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
Re: [PATCH TORQUE-3.1.1] Name Boolean/boolan getters correctly for
generated objects
Posted by Scott Eade <se...@backstagetech.com.au>.
Henning,
I like what this patch does, but I have a couple of concerns. Firstly I
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
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
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
going to sync up with this change in some future release.
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).
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.
3. The Object.vm changes need to be replicated for ObjectWithManager.vm
Scott
--
Scott Eade
Backstage Technologies Pty. Ltd.
http://www.backstagetech.com.au
Henning Schmiedehausen wrote:
>Hi Scott,
>
>I know that you are close to the 3.1.1 release of Torque but I would
>really want you to consider the attached patch. It changes the
>generation of the Getter and Setter methods in the Object.vm and Peer.vm
>to ask the Column. The column object gets two additional getters, called
>getGetterName() and getSetterName(). The getSetterName() just returns
>"set$Property", but the getGetterName() returns "is$property" if the
>property value is either boolean or a java.lang.Boolean.
>
>This is in accordance to the Sun Java Beans rules and I would really
>like to see this patch in the 3.1.1 release.
>
> Regards
> Henning
>
>
>
<snip/>
>Index: src/templates/om/Object.vm
>===================================================================
>RCS file: /home/cvs/db-torque/src/generator/src/templates/om/Object.vm,v
>retrieving revision 1.7.2.3
>diff -u -r1.7.2.3 Object.vm
>--- src/templates/om/Object.vm 14 Aug 2004 12:02:33 -0000 1.7.2.3
>+++ src/templates/om/Object.vm 20 Aug 2004 23:45:29 -0000
>@@ -121,7 +121,7 @@
> *
> * @return $cjtype
> */
>- public $cjtype get${cfc}()
>+ public $cjtype ${col.GetterName} ()
>
>
---------------------------------------^ Why the space here?
> {
> return $clo;
> }
>@@ -143,7 +143,7 @@
> *
> * @param v new value
> */
>- public void set${cfc}($cjtype v) $throwsClause
>+ public void $col.SetterName ($cjtype v) $throwsClause
> {
> #if (($cjtype == "NumberKey") || ($cjtype == "StringKey") || ($cjtype == "DateKey"))
> if (v != null && v.getValue() == null)
>
>
<snip/>
>@@ -501,7 +501,7 @@
> public void add${relColMs}($className l) throws TorqueException
> {
> get${relCol}().add(l);
>- l.set${table.JavaName}${suffix}(($table.JavaName) this);
>+ l.${table.SetterName}${suffix}(($table.JavaName) this);
>
>
-------------^^^^^^^^^^^^^^^^^^ You most likely don't want this change.
> }
>
> /**
>
>
<snip/>
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org