[jbosscache-dev] JBCACHE-867

[Galder Zamarreno]

>> 5.  Cache needs move(), not Node.  The reason is clear if you see 
>> NodeImpl.
>>

>Yes, this is one I have been toying with.  What do others think?

Using move() does seem natural, you're in a node and want to move it and his children to other fqn.

The only downside I can see is people using it from the Cache interface and not realising that this is Node method which might leave them wondering which node you are moving...etc.

I guess a move(Node1, Node2, newFqn) might be easier to read, but this is a method that could well live in Node as it does right now, leaving space for other things in the Cache interface.

Galder Zamarreño
Sr. Software Maintenance Engineer
JBoss, a division of Red Hat

IT executives: Red Hat still #1 for value http://www.redhat.com/promo/vendor/

-----Original Message-----
From: jbosscache-dev-bounces at lists.jboss.org [mailto:jbosscache-dev-bounces at lists.jboss.org] On Behalf Of Manik Surtani
Sent: 20 November 2006 16:43
To: genman at noderunner.net
Cc: jbosscache-dev at lists.jboss.org
Subject: Re: [jbosscache-dev] JBCACHE-867

On 20 Nov 2006, at 04:25, Elias Ross wrote:

>
>
> I finished most of getting rid of the TreeCacheProxyImpl  
> references.  It took
> more time than anticipated.
>
> All said, it was about 1800 lines of adds and 2000 lines of removes.
> TreeNode and DataNode are pretty empty.  There's now a NodeSPI  
> which might
> need some work, but it's pretty pleasant.  I also refactored out a  
> lot of
> lock stuff into something that hangs off of NodeSPI.
>
> Some tests may have been messed up (buddy replication and the move  
> feature
> and probably some others) and I haven't had time to figure these  
> complicated
> ones out.  I hope this doesn't upset anybody.  If somebody could  
> help fix
> these, that would be great.

No this is fine, this was on the cards for me but you've saved me a  
bit of work here.  :-)  Now just to make sure none of the  
functionality's broken.

>
> I would like to suggest the following things:
> 1.  Cache should not implement Node, it's confusing when reading an
> implementation of Cache

We toyed with this quite a bit and in the end we decided to go ahead  
with it since it lets people use the Cache directly rather than to  
get a hold of a Node first.  In what ways do you feel this is confusing?

> 2.  Node sould include useful methods of TreeNode, such as getChild 
> (Object)
> and getName()

There is getChild(Fqn) - and I want to standardise on this.  The Fqn  
passed in is relative to the Node in question so you can have access  
to direct children.

Similarly with getName(), I prefer getFqn().getLast() simply because  
it cements the use of Fqns when it comes to identifying Nodes, and I  
don't think adding a boatload of convenience methods helps the  
interface very much.


> 3.  Cache should have a getCacheSPI()

Again, -1.  I had this in place initially (again as a temp measure)  
and refactored this out.  The purpose of the SPI is to give users  
access to an 'advanced' API which would only be of value to people  
implementing Cache Loaders, Cache Listeners or custom Interceptors.   
Not for typical use of the cache.  And as such, this interface should  
only be injected into such Cache Loaders, Listeners and Interceptors.

> 4.  CacheSPI should not extend Cache

In the case of Cache Loaders, Listeners and Interceptors, they would  
need access to both interfaces.  Don't want to pass in a Cache *and*  
a CacheSPI, and don't want Cache.getCacheSPI() for the reasons  
outlined above.  :-)

> 5.  Cache needs move(), not Node.  The reason is clear if you see  
> NodeImpl.
>

Yes, this is one I have been toying with.  What do others think?


> In the current design, CacheSPI has to have all the methods of  
> Cache.  Why
> couldn't CacheSPI be written as a separate object?  (It could, but  
> then you
> would just have to delegate ...)
>
>
>
> ______________________________________________________________________ 
> ______________
> Sponsored Link
>
> Mortgage rates near 39yr lows.
> $510k for $1,698/mo. Calculate new payment!
> www.LowerMyBills.com/lre
> _______________________________________________
> jbosscache-dev mailing list
> jbosscache-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/jbosscache-dev

_______________________________________________
jbosscache-dev mailing list
jbosscache-dev at lists.jboss.org
https://lists.jboss.org/mailman/listinfo/jbosscache-dev