Ok the more I think about this structure the more useful I think it is for us in many ways.
I'm thinking of making this the main structure to store partitions in the DirectoryService
class rather than using a list of partition configurations. It helps with some of the
refactoring that I would like to do with the configuration.
Also Emmanuel I'd like to see if we can break out the search (lookup) functionality
into a visitor for the tree. As an example you can look at the way this is done with
the filter AST in o.a.d.shared.ldap.filter . Here a visitor can be responsible for adding
new nodes and removing new nodes instead of adding the code to do this into the
node classes themselves.
AlexOn 7/13/07, Alex Karasulu <firstname.lastname@example.org> wrote:Hi Emmanuel,
The code is solid as a rock.
I just worked the clarity of it a bit. Essentially you're building a tree that is
being used as a rapid lookup structure to find partitions corresponding to the
DN of the entries are contained by a partition. I simply renamed a few things
in commit 556176 to make it clear as to what you're doing or at least what I
think you are doing. Notice how much more clear it is when I just renamed
the following classes and moved them into an inner package called
PartitionContainer => BranchNode class
PartitionStructure => Node interface
AbstractPartitionStructure => AbstractNode
PartitionHandler => LeafNode
I hope this was the correct interpretation of what you meant by these names.
Please correct me if I am wrong.
There still are some things that confuse me like how the lookups are done. Could
you add some documentation to describe it? Also it seems you are sometimes using
the partition suffix to check for matching and sometimes using the rdn. Could you
clarify when one is used over the other? Perhaps breaking up complex statements
which you join together like this below will make it easier to understand:
return current.addNode( dn.getRdn( index ).toString(),
buildNode( new BranchNode(), dn, index + 1, partition ) );
Also btw how the index parameter is being used in this recursion is a bit confusing
could you clarify?
I think this data structure might be better integrated directly into the Partition and
PartitionNexus interfaces. Right now you use it as an alternative data structure in the
nexus. Perhaps we could make nexus into a branch node and have it contain other
nexus objects? Meaning a nexus should be what here is modeled as a BranchNode.
A non-nexus Partition can be the equivalent of a LeafNode. This way the data structure
is directly integrated into the architecture rather than having it used in as an alternative
data structure just in the nexus. This way we can gain nested nexus' and use the structure
in an integrated fashion.
Furthermore I know you dislike "patterns" but please try to use the correct terminology
when you are designing them. I know you old timers just do patterns without getting
into the jargon of it all but it helps communicate what you are doing better to us newbies.
People understand tree structures and the language associated with them verses things
like PartitionContainer and PartitionHandler. If this continues I'm going to be totally lost.
On 7/13/07, Emmanuel Lecharny <email@example.com> wrote:You have my +1 for that !
On 7/13/07, Alex Karasulu <firstname.lastname@example.org> wrote:
> I'm looking through this code and it's painful especially with the names
> chosen for the interface,
> and the implementations. I'm going to start reworking it a bit so it can be
> better understood at
> first glance.