commons-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simon Kitching <skitch...@apache.org>
Subject RE: [Digester] enums in ConvertUtils.register
Date Wed, 15 Jun 2005 13:12:33 GMT
Hi Jon,

On Wed, 2005-06-15 at 08:27 -0400, Jon Steelman wrote:
> Right, indirect subclassing of java.lang.Enum is not allowed so any enum
> is an end-of-the-line, but I'll triple check that with the spec. I'll
> explore the listed issues fully, see if I can come up with any others,
> provide some unit testing (is JUnit the preferred way?), and create a
> bugzilla enhancement entry. How's that for service? ;-)  As this is my
> first week using Digester & BeanUtils, where would this code go between
> Digester & BeanUtils and with what class?

That would certainly get you top service on any future support you need
for beanutils and digester :-). And a listing in the contributors
section if you wish.

This functionality definitely belongs in beanutils. Method
  org.apache.commons.beanutils.ConvertUtilsBean.convert(String, Class)
needs to be updated to check for Enum if the initial lookup of converter
based on target class type fails, and before defaulting to
lookup(String.class).

How about creating 

  // dummy class to represent Enum.class; we can't use Enum.class
  // directly as that won't work in pre-1.5
  public class EnumClass {}

and

  public class EnumConverter implements Converter {
    ... the reflection stuff
  }

then updating ConvertUtilsBean.deregister() to add an EnumConverter to
the list of converters by default:

  register(EnumClass.class, new EnumConverter())

then in convert(String, class)

  converter = lookup(clazz)
  if (converter == null) {
    if (clazz is enum) {
      converter = lookup(EnumClass.class);
    }
  }
  if converter == null) {
    converter = lookup(String.class)
  }
  return converter.convert(clazz, value);

Note that ConvertUtilsBean.deregister() is poorly named; it actually
sets the set of registered converters back to the default set.

I don't think we can get away without the hack in method convert, as we
have to test the superclass of the specified class which isn't normal
for convertutils. But this way everything else follows the existing
style. And users can delete or replace the EnumConverter in the
converter list if they wish.

I can't see any backwards compatibility issues here; any custom enum
converters the user has registered would continue to work because we try
a lookup on the specific enum class first. The only time the new code
would be invoked is when the conversion would have produced a String
instead of the requested Enum, and that surely would have produced an
exception anyway. So I think we're ok on that aspect.

Possibly the deregister() method could avoid adding an EnumConverter to
the list at all on pre-1.5 jvms, but I don't see it doing any harm.

And the EnumConverter can throw ConversionException objects when any of
those reflection issues arise which is a nice and tidy solution.

Of course the above is just a suggestion; if you can see a better
approach please say so - or demonstrate the better solution in your
patch.

==

For tests, junit is what all Jakarta projects use; running "maven test"
in the main beanutils directory will run all the junit tests.

You probably just need to add one or two test methods to existing class
src/test/org/apache/commons/beanutils/ConvertUtilsTestCase.java


Good luck!

Simon



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org


Mime
View raw message