dev@javaserverfaces.java.net

[REVIEW] Re: Revist jsf-tools/template-src/TypedCollections.java

From: Ryan Lubke <Ryan.Lubke_at_Sun.COM>
Date: Thu, 12 Oct 2006 13:50:35 -0700

Ryan Lubke wrote:
> Hey All,
>
> I wasn't thinking about performance when reviewing the change bundle
> that included the addition of
> jsf-tools/template-src/TypedCollections.java.
>
> This was added primarily to get around various cast compiler warnings
> that became prevalent when we moved to 1.5.
> My concern is that TypedCollections does an iteration through the
> collection
> to ensure the types of said collection match the input arguments. If not
> then throw a ClassCastException. This can potentially result in two
> iterations
> over the same collection.
>
> Before this was in place, we would just let the ClassCastException happen
> during iteration, so I suggest we go back to doing that and remove the
> type
> validation logic from TypedCollections.
>
> Thoughts?
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>







SECTION: Modified Files
----------------------------
M template-src/TypedCollections.java
 - avoid multiple iterations


SECTION: Diffs
----------------------------
Index: template-src/TypedCollections.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-tools/template-src/TypedCollections.java,v
retrieving revision 1.1
diff -u -r1.1 TypedCollections.java
--- template-src/TypedCollections.java 25 Aug 2006 09:50:19 -0000 1.1
+++ template-src/TypedCollections.java 12 Oct 2006 20:52:03 -0000
@@ -48,16 +48,14 @@
      */
     @SuppressWarnings("unchecked")
     @protection@ static <E,TypedC extends Collection<E>> TypedC dynamicallyCastCollection(Collection<?> c,
- Class<E> type, Class<TypedC> collectionType) {
- if (c == null)
+ Class<E> type,
+ Class<TypedC> collectionType) {
+ if (c == null)
             return null;
         if (!collectionType.isInstance(c))
             throw new ClassCastException(c.getClass().getName());
- for (Object element : c) {
- if (element != null && !type.isInstance(element))
- throw new ClassCastException(element.getClass().getName());
- }
- return collectionType.cast(c);
+
+ return collectionType.cast(c);
     }
 
     /**
@@ -75,7 +73,7 @@
      */
     @SuppressWarnings("unchecked")
     @protection@ static <E> List<E> dynamicallyCastList(List<?> list, Class<E> type) {
- return dynamicallyCastCollection(list, type, List.class);
+ return dynamicallyCastCollection(list, type, List.class);
     }
 
     /**
@@ -92,8 +90,9 @@
      * @throws java.lang.ClassCastException
      */
     @SuppressWarnings("unchecked")
- @protection@ static <E> Set<E> dynamicallyCastSet(Set<?> set, Class<E> type) {
- return dynamicallyCastCollection(set, type, Set.class);
+ @protection@ static <E> Set<E> dynamicallyCastSet(Set<?> set,
+ Class<E> type) {
+ return dynamicallyCastCollection(set, type, Set.class);
     }
 
     /**
@@ -115,16 +114,11 @@
      */
     @SuppressWarnings("unchecked")
     @protection@ static <K, V> Map<K, V> dynamicallyCastMap(Map<?, ?> map,
- Class<K> keyType, Class<V> valueType) {
- if (map != null) {
- for (Map.Entry<?, ?> entry : map.entrySet()) {
- if (entry.getKey() != null && !keyType.isInstance(entry.getKey()))
- throw new ClassCastException(entry.getKey().getClass().getName());
- if (entry.getValue() != null
- && !valueType.isInstance(entry.getValue()))
- throw new ClassCastException(entry.getValue().getClass().getName());
- }
- }
- return (Map<K, V>) map;
+ Class<K> keyType,
+ Class<V> valueType) {
+ if (map == null) {
+ return null;
+ }
+ return (Map<K, V>) map;
     }
 }