On Oct 23, 2010, at 11:59 AM, Les Hazlewood wrote:

> But I'm not sure that inheritance is used too often.  Sure, it is used
> in some places where it shouldn't be and that there are definitely
> places where we can and should eliminate it.  For example, the
> CodecSupport class and the HashedCredentialsMatcher hierarchy can be
> deprecated now and then removed in a 2.0 release (to retain backwards
> compatibility).

Side comment, it's used all over the place.  Filters, etc.

> But if you look at pretty much all of Shiro's inheritance mechanisms
> (SecurityManager and its subclasses, Realm and its subclasses,
> SessionManager and its subclasses, etc), almost _all_ of those things
> are purely wrapper classes that delegate to internal pluggable
> components.  Almost all of it is composition.
> 
> These class hierarchies for these things exist purely to logically
> separate behavior associated with respective state (the internal
> delegates).  If we didn't do this for example, the SecurityManager
> (which mostly uses delegation for 95% of its logic) would be a _huge_
> class - probably well over 3000 lines long.

Let's look at the DefaultSecurityManager.  DefaultSecurityManager extends 
SessionsSecurityManager,  SessionsSecurityManager extends 
AuthorizingSecurityManager,  AuthorizingSecurityManager extends 
AuthenticatingSecurityManager,  AuthenticatingSecurityManager extends 
RealmSecurityManager, and, finally, RealmSecurityManager extends 
CachingSecurityManager.

I'm guessing there's over 1200 lines of code there spread across six classes.

All this for wrapper classes.

Don't get me wrong.  I'm all for delegation.  What I am against is the long 
chain of inheritance.  The above inheritance list reads like Matthew chapter 1. 
 ;)  Why such a large totem pole?  I haven't groked the entire set of code but 
I suspect there's some intimate collaboration there.  If there isn't then they 
really should be broken up.

They should be broken up anyway.

Take this with a grain of salt; as I mentioned before I don't grok the entire 
set of the above code.  The managers should be separate final entities.  
Collaborative behavior could be specified by wiring in various listeners 
between them.  The DefaultSecurityManager could do this.



Regards,
Alan
P.S. Go Giants!


Reply via email to