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 Regards, Pierre-Arnaud
