You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by "Paul Stanton (JIRA)" <ji...@apache.org> on 2011/02/26 23:47:58 UTC
[jira] Created: (TAP5-1452) messages property lookup for enum
values is inconsistent, and incorrect
messages property lookup for enum values is inconsistent, and incorrect
-----------------------------------------------------------------------
Key: TAP5-1452
URL: https://issues.apache.org/jira/browse/TAP5-1452
Project: Tapestry 5
Issue Type: Bug
Components: tapestry-core
Affects Versions: 5.1.0.7, 5.2.4
Reporter: Paul Stanton
Priority: Minor
I work on a project which has a few different entities each with their own specific ranges of potential values for 'Category'. The way this project has achieved this is to inline Enums within the entity class. Unfortunately, the classes all define an enum named "Category", ie:
public class EntityOne
{
enum Category {...};
private Category category;
....
}
public class EntityTwo
{
enum Category {...};
private Category category;
....
}
The EnumSelectModel uses TapestryInternalUtils.getLabelForEnum(Messages messages, String prefix, Enum value) where prefix is enumClass.getSimpleName()
This makes it impossible to differentiate inline enums in different classes with the same name since EntityOne.Category and EntityTwo.Category would both have the prefix "Category".
After doing some further digging I also found that there was another way to resolve a message for an enum value: TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value) which is used by PropertyDisplayBlocks. This method is slightly better since it would look for EntityOne$Category instead of just Category, however I think it would be better still to use the fully qualified name.
I think it is important to
a) be consistent
b) allow for all cases
Here is my solution:
1. scrap TapestryInternalUtils.getLabelForEnum(Messages messages, String prefix, Enum value) and move the cascading logic into TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value), update EnumSelectModel to use TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value).
2. change the cascading lookup logic to
a) test for [value.getClass().getName() + "." + value.name()]
b) test for [lastTerm(value.getClass().getName()) + "." + value.name()]
c) test for [value.getClass().getSimpleName() + "." + value.name()]
d) test for [value.name()]
The code:
public static String getLabelForEnum(Messages messages, Enum value)
{
String key1 = value.getClass().getName() + "." + value.name();
if (messages.contains(key1))
return messages.get(key1);
String key2 = lastTerm(value.getClass().getName()) + "." + value.name();
if (messages.contains(key2))
return messages.get(key2);
String key3 = value.getClass().getSimpleName() + "." + value.name();
if (messages.contains(key3))
return messages.get(key3);
String key4 = value.name();
if (messages.contains(key4))
return messages.get(key4);
return toUserPresentable(value.name().toLowerCase());
}
EnumSelectModel and PropertyDisplayBlocks could use this without issue and be fully backwards compatible with previously defined message properties.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (TAP5-1452) messages property lookup for enum
values is inconsistent, and incorrect
Posted by "Howard M. Lewis Ship (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-1452?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13113560#comment-13113560 ]
Howard M. Lewis Ship commented on TAP5-1452:
--------------------------------------------
Might make this change if you provide a patch with tests.
Have you considered moving the enums out as top-level classes?
> messages property lookup for enum values is inconsistent, and incorrect
> -----------------------------------------------------------------------
>
> Key: TAP5-1452
> URL: https://issues.apache.org/jira/browse/TAP5-1452
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.2.4, 5.1.0.7
> Reporter: Paul Stanton
> Priority: Minor
>
> I work on a project which has a few different entities each with their own specific ranges of potential values for 'Category'. The way this project has achieved this is to inline Enums within the entity class. Unfortunately, the classes all define an enum named "Category", ie:
> public class EntityOne
> {
> enum Category {...};
> private Category category;
> ....
> }
> public class EntityTwo
> {
> enum Category {...};
> private Category category;
> ....
> }
> The EnumSelectModel uses TapestryInternalUtils.getLabelForEnum(Messages messages, String prefix, Enum value) where prefix is enumClass.getSimpleName()
> This makes it impossible to differentiate inline enums in different classes with the same name since EntityOne.Category and EntityTwo.Category would both have the prefix "Category".
> After doing some further digging I also found that there was another way to resolve a message for an enum value: TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value) which is used by PropertyDisplayBlocks. This method is slightly better since it would look for EntityOne$Category instead of just Category, however I think it would be better still to use the fully qualified name.
> I think it is important to
> a) be consistent
> b) allow for all cases
> Here is my solution:
> 1. scrap TapestryInternalUtils.getLabelForEnum(Messages messages, String prefix, Enum value) and move the cascading logic into TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value), update EnumSelectModel to use TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value).
> 2. change the cascading lookup logic to
> a) test for [value.getClass().getName() + "." + value.name()]
> b) test for [lastTerm(value.getClass().getName()) + "." + value.name()]
> c) test for [value.getClass().getSimpleName() + "." + value.name()]
> d) test for [value.name()]
> The code:
> public static String getLabelForEnum(Messages messages, Enum value)
> {
> String key1 = value.getClass().getName() + "." + value.name();
> if (messages.contains(key1))
> return messages.get(key1);
> String key2 = lastTerm(value.getClass().getName()) + "." + value.name();
> if (messages.contains(key2))
> return messages.get(key2);
> String key3 = value.getClass().getSimpleName() + "." + value.name();
> if (messages.contains(key3))
> return messages.get(key3);
> String key4 = value.name();
> if (messages.contains(key4))
> return messages.get(key4);
> return toUserPresentable(value.name().toLowerCase());
> }
> EnumSelectModel and PropertyDisplayBlocks could use this without issue and be fully backwards compatible with previously defined message properties.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (TAP5-1452) messages property lookup for enum
values is inconsistent, and incorrect
Posted by "Paul Stanton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-1452?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12999852#comment-12999852 ]
Paul Stanton commented on TAP5-1452:
------------------------------------
Also, it's probably a good idea to return "" or null if value parameter is null.
> messages property lookup for enum values is inconsistent, and incorrect
> -----------------------------------------------------------------------
>
> Key: TAP5-1452
> URL: https://issues.apache.org/jira/browse/TAP5-1452
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.2.4, 5.1.0.7
> Reporter: Paul Stanton
> Priority: Minor
>
> I work on a project which has a few different entities each with their own specific ranges of potential values for 'Category'. The way this project has achieved this is to inline Enums within the entity class. Unfortunately, the classes all define an enum named "Category", ie:
> public class EntityOne
> {
> enum Category {...};
> private Category category;
> ....
> }
> public class EntityTwo
> {
> enum Category {...};
> private Category category;
> ....
> }
> The EnumSelectModel uses TapestryInternalUtils.getLabelForEnum(Messages messages, String prefix, Enum value) where prefix is enumClass.getSimpleName()
> This makes it impossible to differentiate inline enums in different classes with the same name since EntityOne.Category and EntityTwo.Category would both have the prefix "Category".
> After doing some further digging I also found that there was another way to resolve a message for an enum value: TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value) which is used by PropertyDisplayBlocks. This method is slightly better since it would look for EntityOne$Category instead of just Category, however I think it would be better still to use the fully qualified name.
> I think it is important to
> a) be consistent
> b) allow for all cases
> Here is my solution:
> 1. scrap TapestryInternalUtils.getLabelForEnum(Messages messages, String prefix, Enum value) and move the cascading logic into TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value), update EnumSelectModel to use TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value).
> 2. change the cascading lookup logic to
> a) test for [value.getClass().getName() + "." + value.name()]
> b) test for [lastTerm(value.getClass().getName()) + "." + value.name()]
> c) test for [value.getClass().getSimpleName() + "." + value.name()]
> d) test for [value.name()]
> The code:
> public static String getLabelForEnum(Messages messages, Enum value)
> {
> String key1 = value.getClass().getName() + "." + value.name();
> if (messages.contains(key1))
> return messages.get(key1);
> String key2 = lastTerm(value.getClass().getName()) + "." + value.name();
> if (messages.contains(key2))
> return messages.get(key2);
> String key3 = value.getClass().getSimpleName() + "." + value.name();
> if (messages.contains(key3))
> return messages.get(key3);
> String key4 = value.name();
> if (messages.contains(key4))
> return messages.get(key4);
> return toUserPresentable(value.name().toLowerCase());
> }
> EnumSelectModel and PropertyDisplayBlocks could use this without issue and be fully backwards compatible with previously defined message properties.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (TAP5-1452) messages property lookup for enum
values is inconsistent, and incorrect
Posted by "Howard M. Lewis Ship (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-1452?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13113560#comment-13113560 ]
Howard M. Lewis Ship commented on TAP5-1452:
--------------------------------------------
Might make this change if you provide a patch with tests.
Have you considered moving the enums out as top-level classes?
> messages property lookup for enum values is inconsistent, and incorrect
> -----------------------------------------------------------------------
>
> Key: TAP5-1452
> URL: https://issues.apache.org/jira/browse/TAP5-1452
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.2.4, 5.1.0.7
> Reporter: Paul Stanton
> Priority: Minor
>
> I work on a project which has a few different entities each with their own specific ranges of potential values for 'Category'. The way this project has achieved this is to inline Enums within the entity class. Unfortunately, the classes all define an enum named "Category", ie:
> public class EntityOne
> {
> enum Category {...};
> private Category category;
> ....
> }
> public class EntityTwo
> {
> enum Category {...};
> private Category category;
> ....
> }
> The EnumSelectModel uses TapestryInternalUtils.getLabelForEnum(Messages messages, String prefix, Enum value) where prefix is enumClass.getSimpleName()
> This makes it impossible to differentiate inline enums in different classes with the same name since EntityOne.Category and EntityTwo.Category would both have the prefix "Category".
> After doing some further digging I also found that there was another way to resolve a message for an enum value: TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value) which is used by PropertyDisplayBlocks. This method is slightly better since it would look for EntityOne$Category instead of just Category, however I think it would be better still to use the fully qualified name.
> I think it is important to
> a) be consistent
> b) allow for all cases
> Here is my solution:
> 1. scrap TapestryInternalUtils.getLabelForEnum(Messages messages, String prefix, Enum value) and move the cascading logic into TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value), update EnumSelectModel to use TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value).
> 2. change the cascading lookup logic to
> a) test for [value.getClass().getName() + "." + value.name()]
> b) test for [lastTerm(value.getClass().getName()) + "." + value.name()]
> c) test for [value.getClass().getSimpleName() + "." + value.name()]
> d) test for [value.name()]
> The code:
> public static String getLabelForEnum(Messages messages, Enum value)
> {
> String key1 = value.getClass().getName() + "." + value.name();
> if (messages.contains(key1))
> return messages.get(key1);
> String key2 = lastTerm(value.getClass().getName()) + "." + value.name();
> if (messages.contains(key2))
> return messages.get(key2);
> String key3 = value.getClass().getSimpleName() + "." + value.name();
> if (messages.contains(key3))
> return messages.get(key3);
> String key4 = value.name();
> if (messages.contains(key4))
> return messages.get(key4);
> return toUserPresentable(value.name().toLowerCase());
> }
> EnumSelectModel and PropertyDisplayBlocks could use this without issue and be fully backwards compatible with previously defined message properties.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (TAP5-1452) messages property lookup for enum
values is inconsistent, and incorrect
Posted by "Paul Stanton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/TAP5-1452?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12999852#comment-12999852 ]
Paul Stanton commented on TAP5-1452:
------------------------------------
Also, it's probably a good idea to return "" or null if value parameter is null.
> messages property lookup for enum values is inconsistent, and incorrect
> -----------------------------------------------------------------------
>
> Key: TAP5-1452
> URL: https://issues.apache.org/jira/browse/TAP5-1452
> Project: Tapestry 5
> Issue Type: Bug
> Components: tapestry-core
> Affects Versions: 5.2.4, 5.1.0.7
> Reporter: Paul Stanton
> Priority: Minor
>
> I work on a project which has a few different entities each with their own specific ranges of potential values for 'Category'. The way this project has achieved this is to inline Enums within the entity class. Unfortunately, the classes all define an enum named "Category", ie:
> public class EntityOne
> {
> enum Category {...};
> private Category category;
> ....
> }
> public class EntityTwo
> {
> enum Category {...};
> private Category category;
> ....
> }
> The EnumSelectModel uses TapestryInternalUtils.getLabelForEnum(Messages messages, String prefix, Enum value) where prefix is enumClass.getSimpleName()
> This makes it impossible to differentiate inline enums in different classes with the same name since EntityOne.Category and EntityTwo.Category would both have the prefix "Category".
> After doing some further digging I also found that there was another way to resolve a message for an enum value: TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value) which is used by PropertyDisplayBlocks. This method is slightly better since it would look for EntityOne$Category instead of just Category, however I think it would be better still to use the fully qualified name.
> I think it is important to
> a) be consistent
> b) allow for all cases
> Here is my solution:
> 1. scrap TapestryInternalUtils.getLabelForEnum(Messages messages, String prefix, Enum value) and move the cascading logic into TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value), update EnumSelectModel to use TapestryInternalUtils.getLabelForEnum(Messages messages, Enum value).
> 2. change the cascading lookup logic to
> a) test for [value.getClass().getName() + "." + value.name()]
> b) test for [lastTerm(value.getClass().getName()) + "." + value.name()]
> c) test for [value.getClass().getSimpleName() + "." + value.name()]
> d) test for [value.name()]
> The code:
> public static String getLabelForEnum(Messages messages, Enum value)
> {
> String key1 = value.getClass().getName() + "." + value.name();
> if (messages.contains(key1))
> return messages.get(key1);
> String key2 = lastTerm(value.getClass().getName()) + "." + value.name();
> if (messages.contains(key2))
> return messages.get(key2);
> String key3 = value.getClass().getSimpleName() + "." + value.name();
> if (messages.contains(key3))
> return messages.get(key3);
> String key4 = value.name();
> if (messages.contains(key4))
> return messages.get(key4);
> return toUserPresentable(value.name().toLowerCase());
> }
> EnumSelectModel and PropertyDisplayBlocks could use this without issue and be fully backwards compatible with previously defined message properties.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira