[REVIEW] Fix for spec issue 172

From: Robertson, Tony <>
Date: Wed, 16 Aug 2006 16:28:53 +1000

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
then calling "targetClass.isInstance(value)" is sufficient.
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
(which is therefore part of the JSF-API spec) is describing an
instead of describing a behaviour.

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

<p>Convert the enum constant given by the <code>value</code>
argument into a String. If no target class argument has been
provided to the constructor of this instance, throw a
<code>ConverterException</code> containing the
{_at_link #ENUM_NO_CLASS_ID} message with proper parameters.
If the <code>value</code> argument is <code>null</code>,
return <code>null</code>.
If the value is an instance of the provided target class,
Return its string value (<code>value.toString()</code>).
Otherwise, throw a {_at_link ConverterException} containing the
{_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]
> Sent: Tuesday, August 15, 2006 2:55 PM
> To:
> Subject: [REVIEW] Fix for spec issue 172
> Changes attached to issue.