RE: Re: [REVIEW] Fix for spec issue 172

From: Jason Lee <>
Date: Thu, 17 Aug 2006 15:59:52 -0500

I can't comment on the issue tracker, so I'll do it here.
The bundle looks good to me, with the exception of the HTML errors in
the javadoc (,/code> instead of </code>). Assuming that's corrected
(minor as it is)
Jason Lee
Programmer/Analyst <>


        From: Roger.Kitain_at_Sun.COM [mailto:Roger.Kitain_at_Sun.COM]
        Sent: Wednesday, August 16, 2006 10:44 AM
        Subject: Re: [REVIEW] Fix for spec issue 172
        I like the verbage and approach.
        I've posted another change bundle.
        Robertson, Tony wrote:

                I also don't understand why the
"targetClass.getEnumConstants()" is used at all.
                If you want to check that the "value" really is one of
the enum constants,
                then calling "targetClass.isInstance(value)" is
                Given that all enums are effectively "final" classes
with only private
                constructors, any instance of an enum class must also be
a member of the
                "enumConstants" list.
                This seems to be another case where the javadoc comment
for this interface
                (which is therefore part of the JSF-API spec) is
describing an implementation
                instead of describing a behaviour.

                If you are going to change the javadoc (and therefore
the API spec) I would
                rather see a clear description of the required
"behaviour" only.
                Perhaps something more like this:

                <p>Convert the enum constant given by the
                argument into a String. If no target class argument has
                provided to the constructor of this instance, throw a
                <code>ConverterException</code> containing the
                {_at_link #ENUM_NO_CLASS_ID} message with proper
                If the <code>value</code> argument is <code>null</code>,

                return <code>null</code>.
                If the value is an instance of the provided target
                Return its string value (<code>value.toString()</code>).

                Otherwise, throw a {_at_link ConverterException} containing
                {_at_link #ENUM_ID} message with proper parameters.</p>

                Date: Tue, 15 Aug 2006 15:40:30 -0500
                From: Jason Lee <> <>

                Subject: [REVIEW] Fix for spec issue 172

                Just out of curiosity, why use the break, then the test?
Why not do

                        for (int i=0; i<enumConstants.length; i++) {
                            if (enumConstants[i].equals(value)) {
                                return value.toString();
                        // No match found, so throw an exception
                        throw new ConverterException(

                Jason Lee

> -----Original Message-----
> From: Roger.Kitain_at_Sun.COM
[mailto:Roger.Kitain_at_Sun.COM <mailto:Roger.Kitain_at_Sun.COM> ]
> Sent: Tuesday, August 15, 2006 2:55 PM
> To:
> Subject: [REVIEW] Fix for spec issue 172
> Changes attached to issue.