directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pierre-Arnaud Marcelot>
Subject Re: ACITuple refactoring
Date Fri, 02 Jul 2010 08:46:22 GMT
Hi Emmanuel,

On 2 juil. 2010, at 09:42, Emmanuel Lecharny wrote:

> Hi guys,
> I'm going deeper into the ACI review. I have done some refactoring :
> - The ProtectedItem class now does not contain all the ProtectedItem subclasses anymore,
each one of those classes has now its own Java class
> - ACIItemParser is Schema aware. That means we don't manipulate AttributeType as String.

These are two good things... :)

> There are a few more things I want to do:
> 1) ACITuple constructor takes 6 parameters. I do think it's way too many, and I'd like
to either use setters (but that would make the class mutable) or define a factory for tuples.
> 2) The ACDFEngine checkPermission() and hasPermission() methods, plus the ACITupleFilter
filter() operations take 14 (!!!) parameters. I think we should refactor those methods to
take a data structure instead, because it's really difficult to debug what's going on, assuming
that depending on the filter, some of the filter's parameters are null, because useless.
> 3) The checkPermission() and hasPermission() methods are most certainly doing the same
thing, I will remove one of them.

+1 for the introduction of a simplified constructor (maybe with not parameter at all) and
use setters instead.
Same thing for the methods with the idea of a data structure.
This is definitely cleaner.
I looked at the interface and I think it's the larger method signature I've ever seen... :D

View raw message