API Design

a few random thoughts about your api design, to make things a bit more consistent.. feel free to add stuff

lists of things

Preferring Collection<..> over iterables, array objects, etc. where applicable

We started off having many Iterable<...> or array objects, but it's a bit annoying to not have e.g. .size(), etc. AbstractCollection already implements everything anyway which is basically an Iterable with a .size() attribute..

null checks

i (HP) currently try to use javax.annotation.Nonnull and Nullable in places which make sense, and enable eclipse's null checks likeways. (= JSR-305 which is already dead) - later it makes probably sense to switch over to java 8 which implements JSR-308 - it seems it has improved annotation support.. but not sure yet how it actually improves null checks.. but see http://types.cs.washington.edu/checker-framework/ (checker framework) and it seems the latest JDT null checking is also based on JSR-308 (not to be released stable until march 2014): https://wiki.eclipse.org/JDT_Core/Java8 "JSR308 type annotations based null analysis (substantially complete - some open issues exist)"

no internal getters

when not within interfaces, but accessing properties between classes within one .impl. package, just make the property package-level visible and access it directly, no need for Foo getFoo() { return foo; } .. except obviously if it's a real customer-facing API.

naming

this is probably something we should get standardize asap :-)

getters

listNodes() vs. getNodes() vs. loadNodes() vs. getNodeList() vs. queryNodesBy, etc.. we probably only need a distiction between just getting a state and actually performing a operation in the background (imo listNodes() makes it more obvious that there is some kind of loading operation than getNodes())

my suggestion:

• listNodes/listChildren if there is a potential load operation
• getNodes/getChildren if it just returns a state (already loaded nodes)

(although i'm not even sure this distinction makes even sense, as this is more implementation specific and shouldn't be dictated by the API?)

instantiation/creation methods

for children / new nodes, etc. we've now always used add

• addChild() # creates a node and adds it as child
  • addChild(name)
• addNodeType() # creates a node type and adds it

returning null vs. throwing exceptions?

example: NodeTypeAccessor.addNodeTypeDefinition - i've decided to throw an IllegalArgumentException rather than returning null, when the node type can't be added, because another with the same name already exists.

• pro: we make sure no null is ever returned, which should prevent NPEs
• contra: it's an unchecked exception, so it's possible that the NPE is replaced by an uncatched IllegalArgumentException .. does this make things better?
  • IMHO (FG) unchecked exceptions make things way better than NPE. You fail as early as possible, always see immediately the root of the problem and have a specialized error message.

constants in tests?

when using constants (e.g. 'name', ':name', ':nodetype' should we use the literal string in tests, or the static String property?)