incubator-directmemory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benoit Perroud <ben...@noisette.ch>
Subject Re: svn commit: r1293572 - in /incubator/directmemory/trunk: directmemory-cache/src/main/java/org/apache/directmemory/cache/ directmemory-cache/src/main/java/org/apache/directmemory/memory/ directmemory-cache/src/test/java/org/apache/directmemory/mem
Date Sat, 25 Feb 2012 14:09:27 GMT
Hi Simone,

Thanks for your comment.

I'm working on an iterative process to refactor the backend but I
don't want to break everything and reincarnate in stone ;)
So here I just wanted to extract the interface, it's now easier to
refactor starting from here.

All your remarks are really relevant and I will take them in account.
I should be able to preview something in a fork by the begining of the
week.

Thanks !

Benoit.

2012/2/25 Simone Tripodi <simonetripodi@apache.org>:
> Salut Benoit!
>
> merci, I like it! Since we are changing the Pointer design, I just
> have few questions - I see you replicated the original behavior ATM -
> so I am asking about possible improvements, I commented inline
>
>> +public interface Pointer<T>
>> +{
>> +
>> +    byte[] content();
>>
>> -import java.nio.ByteBuffer;
>> +    boolean isFree();
>>
>> -public class Pointer<T>
>> -{
>> -    public int start;
>> +    void setFree( boolean free );
>>
>
> I don't understand why the pointer free status should be set from
> outside - shouldn't be a state set internally and read from outside?
>
>> -    public int end;
>> +    boolean isExpired();
>>
>> -    public long created;
>> +    float getFrequency();
>>
>> -    public long expires;
>> +    int getCapacity();
>>
>> -    public long expiresIn;
>> +    void reset();
>>
>> -    public long hits;
>> +    int getBufferNumber();
>>
>> -    public boolean free;
>> +    void setBufferNumber( int bufferNumber );
>>
>
> can't the bufferNumber be immutable? what does it mean change it at
> runtime, while the cache is running?
>
>> -    public long lastHit;
>> +    int getStart();
>>
>> -    public int bufferNumber;
>> +    void setStart( int start );
>>
>> -    public Class<? extends T> clazz;
>> +    int getEnd();
>>
>> -    public ByteBuffer directBuffer = null;
>> +    void setEnd( int end );
>>
>
> as above: couldn't start/end be immutable?
>
>> -    public Pointer()
>> -    {
>> -    }
>> +    void hit();
>>
>> -    public Pointer( int start, int end )
>> -    {
>> -        this.start = start;
>> -        this.end = end;
>> -    }
>> +    Class<? extends T> getClazz();
>>
>> -    public byte[] content()
>> -    {
>> -        return null;
>> -    }
>> +    void setClazz( Class<? extends T> clazz );
>>
>
> as above: shouldn't the reflected type be immutable? or the
> application needs to be able to reset the type?
>
>> -    public boolean expired()
>> -    {
>> -        if ( expires > 0 || expiresIn > 0 )
>> -        {
>> -            return ( expiresIn + created < currentTimeMillis() );
>> -        }
>> -        return false;
>> -    }
>> +    ByteBuffer getDirectBuffer();
>>
>> -    public float getFrequency()
>> -    {
>> -        return (float) ( currentTimeMillis() - created ) / hits;
>> -    }
>> +    void setDirectBuffer( ByteBuffer directBuffer );
>>
>
> as above
>
>> -    public int getCapacity()
>> -    {
>> -        return end - start + 1;
>> -    }
>> +    void createdNow();
>> +
>> +    void setExpiration( long expires, long expiresIn );
>>
>> -    @Override
>> -    public String toString()
>> -    {
>> -        return format( "%s[%s, %s] %s free", getClass().getSimpleName(), start,
end,( free ? "" : "not" ) );
>> -    }
>>  }
>>
>
> IMHO the Pointer interface should expose read-only methods as much as
> possible, and set them in the related Impl via the constructor.
>
> WDYT? Merci en avance,
> Simo



-- 
sent from my Nokia 3210

Mime
View raw message