Indeed, I AM WRONG :D

http://people.apache.org/~simonetripodi/
http://simonetripodi.livejournal.com/
http://twitter.com/simonetripodi
http://www.99soft.org/



On Tue, Feb 21, 2012 at 8:20 AM, Simone Tripodi
<[email protected]> wrote:
> Hi again,
>
> +    public static Serializer createNewSerializer( String serializerClassName 
> )
> +    {
> +        Class<?> serializerClass;
> +        try
> +        {
> +            serializerClass = Class.forName( serializerClassName );
> +        }
> +        catch ( ClassNotFoundException e )
> +        {
> +            return null;
> +        }
> +
> +        if ( serializerClass.isAssignableFrom( Serializer.class ) )
>
> this condition block won't ever reached. it the try{} block fails, the
> method returns null.
>
> please don't get me wrong, I am not pedantic :D
> -Simo
>
> http://people.apache.org/~simonetripodi/
> http://simonetripodi.livejournal.com/
> http://twitter.com/simonetripodi
> http://www.99soft.org/
>
>
>
> On Tue, Feb 21, 2012 at 8:15 AM, Simone Tripodi
> <[email protected]> wrote:
>> Hello,
>>
>>> +                Serializer next = serializers.next();
>>> +                if ( next.getClass().getName().equals( 
>>> serializer.getName() ) )
>>> +                {
>>> +                    return serializer.cast( next );
>>> +                }
>>
>> "trivial", but I am going to fix:
>>
>>  * expected values in assertions should be put at the beginning;
>>  * string comparison for classes is not really canonical. I'm going to
>> change it to 
>> Class.isInstance(Serializer)<http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/Class.html#isInstance(java.lang.Object)>
>>
>>> +    public static Serializer createNewSerializer( String 
>>> serializerClassName )
>>> +    {
>>> +        Class<?> serializerClass;
>>> +        try
>>> +        {
>>> +            serializerClass = Class.forName( serializerClassName );
>>
>> Class.forName is evil for OSGi mates and would complain about it, I'm
>> going to add a new method
>>
>> +--------+
>> public static Serializer createNewSerializer( String
>> serializerClassName, ClassLoader loader )
>> +--------+
>>
>> and the default one will use the SerializerFactory.class.getClassLoader().
>>
>>> +        }
>>> +        catch ( ClassNotFoundException e )
>>> +        {
>>> +            return null;
>>> +        }
>>> +
>>> +        if ( serializerClass.isAssignableFrom( Serializer.class ) )
>>> +        {
>>> +            Iterator<Serializer> serializers = load( Serializer.class 
>>> ).iterator();
>>> +
>>> +            // iterate over all found services
>>> +            while ( serializers.hasNext() )
>>> +            {
>>> +                // try getting the current service and return
>>> +                try
>>> +                {
>>> +                    Serializer next = serializers.next();
>>> +                    if ( next.getClass().getName().equals( 
>>> serializerClassName ) )
>>> +                    {
>>> +                        return next;
>>> +                    }
>>> +                }
>>> +                catch ( Throwable t )
>>> +                {
>>> +                    // just ignore, skip and try getting the next
>>> +                }
>>> +            }
>>> +        }
>>> +
>>> +        return null;
>>> +    }
>>
>> this is exactly the <S extends Serializer> S createNewSerializer(
>> Class<S> serializer ) method invocation, no needs to repeat the same
>> code.
>>
>> best,
>> -Simo
>>
>> http://people.apache.org/~simonetripodi/
>> http://simonetripodi.livejournal.com/
>> http://twitter.com/simonetripodi
>> http://www.99soft.org/
>>
>>
>>
>> On Mon, Feb 20, 2012 at 10:41 PM,  <[email protected]> wrote:
>>> Author: olamy
>>> Date: Mon Feb 20 21:41:42 2012
>>> New Revision: 1291458
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1291458&view=rev
>>> Log:
>>> [DIRECTMEMORY-67] Serializer Factory should be able to load specific 
>>> serializers
>>> Submitted by Daniel Manzke.
>>>
>>> Modified:
>>>    
>>> incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/serialization/SerializerFactory.java
>>>
>>> Modified: 
>>> incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/serialization/SerializerFactory.java
>>> URL: 
>>> http://svn.apache.org/viewvc/incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/serialization/SerializerFactory.java?rev=1291458&r1=1291457&r2=1291458&view=diff
>>> ==============================================================================
>>> --- 
>>> incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/serialization/SerializerFactory.java
>>>  (original)
>>> +++ 
>>> incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/serialization/SerializerFactory.java
>>>  Mon Feb 20 21:41:42 2012
>>> @@ -19,10 +19,10 @@ package org.apache.directmemory.serializ
>>>  * under the License.
>>>  */
>>>
>>> -import static java.util.ServiceLoader.load;
>>> -
>>>  import java.util.Iterator;
>>>
>>> +import static java.util.ServiceLoader.load;
>>> +
>>>  public final class SerializerFactory
>>>  {
>>>
>>> @@ -47,6 +47,69 @@ public final class SerializerFactory
>>>         return new StandardSerializer();
>>>     }
>>>
>>> +    public static <S extends Serializer> S createNewSerializer( Class<S> 
>>> serializer )
>>> +    {
>>> +        Iterator<Serializer> serializers = load( Serializer.class 
>>> ).iterator();
>>> +
>>> +        // iterate over all found services
>>> +        while ( serializers.hasNext() )
>>> +        {
>>> +            // try getting the current service and return
>>> +            try
>>> +            {
>>> +                Serializer next = serializers.next();
>>> +                if ( next.getClass().getName().equals( 
>>> serializer.getName() ) )
>>> +                {
>>> +                    return serializer.cast( next );
>>> +                }
>>> +            }
>>> +            catch ( Throwable t )
>>> +            {
>>> +                // just ignore, skip and try getting the next
>>> +            }
>>> +        }
>>> +
>>> +        return null;
>>> +    }
>>> +
>>
>>> +
>>>     /**
>>>      * Hidden constructor, this class cannot be instantiated
>>>      */
>>>
>>>

Reply via email to