Adrian Crum wrote: > --- On Fri, 2/12/10, Adam Heath <[email protected]> wrote: >> From: Adam Heath <[email protected]> >> Subject: Re: svn commit: r908676 - >> /ofbiz/trunk/framework/base/src/org/ofbiz/base/test/BaseUnitTests.java >> To: [email protected] >> Date: Friday, February 12, 2010, 10:16 AM >> Adam Heath wrote: >>> Adrian Crum wrote: >>>> Actually, that error message was caused by rev >> 908700. >>> How? That added an Enum converter; I didn't >> think ofbiz actually used >>> enums mutch. >> Ok, I see it; Is this code trying to convert a String to an >> Enum? >> That's not supported. To make that work, you also >> need to pass in a >> the target class. Perhaps we should remove the single >> arg variant of >> convert. > > Why would we want to do that? Also, why not extend > AbstractConverter like the other classes? The exception > goes away if the converter is written like the rest. Also, > StringToEnum.convert(String obj) should throw ConversionException.
I choose not to extend AbstractConverter, because StringToEnum requires a concrete class, it can't just pick any random class that happens to extend Enum. That's why I added convert(targetClass, obj) variants. It's also why I throw UnsupportedOperationException, instead of ConversionException, during convert(obj). You shouldn't do doing unknown conversions, when dealing with enums. In those cases, you should know that an enum is being requested, and use the other convert method. While tracking this one down, I discovered that the url in question was eventually calling into ObjectType, and the conversion framework, and was trying to do a String->Object conversion; that would never work, and is stupid to do. The bug that I fixed in 909718 made the StringToEnum converter think it could handle that request. Also, this commit allows me to possibly remove the checkExtendsImplements method. > > > > >
