GraphSchema cleanup - Githubissues

We have dynamicLabelDefinitions and staticLabelDefinitions. staticLabelDefinitions seems unused. Remove it?

We should probably just have labelDefinitions and remove the static/dynamic division. Static was used for vertex label definitions that were created in code using new statements. Dynamic was used to represent definitions that were auto-generated from code. We now just focus on the latter. The static/dynamic verbiage is confusing.

We have an interface GraphSchema. Do we want to keep it? If so, rename CoreGraphSchema to DefaultGraphSchema?

Assuming we remove the static/dynamic division as per above, then CoreGraphSchema would be dead simple -- just variables with default get/set methods. There seems to be an obvious reason to have an interface for GraphValidator as the implementation of each validator could vary based on the underlying graph database. GraphSchema is unlikely to have many implementations. However, GraphSchema objects should be immutable. Perhaps it makes sense to keep GraphSchema interface with just get methods to reflect this.

The term controlledInstance is still thrown around. Standardize on vertexBuilder?

getVertexBuilders() seems too vague. Controlled instances is better, but also seems vague. Controlled instances should probably be renamed to singleton vertices. That would be specific, perhaps in a way that borders on too limiting, but it would be a more accurate reflection of what the code does and probably as a result more relevant to application programmers.

GraphSchema stores all the VertexBuilders, but not the EdgeBuilders. Is there a reason EdgeBuilders are left out?

Yes, there is a reason. Collection<VertexBuilder> getVertexBuilders() is used to store builders for "controlled instances". Controlled instances are singleton vertices that are governed by rules of cardinality and required properties. They are used to instantiate vertices that represent classes, for example. There is no analog for edges, so GraphSchema has no need of a getEdgeBuilders() method.

GraphSchema contains getVertexBuilders() while CoreGraphSchema, which implements GraphSchema, has a controlledInstances property to which getVertexBuilders() is mapped. It does not seem that getVertexBuilders() is used in the code anywhere.

See above on why we should rename getVertexBuilders().

Keep RelationshipDefinition?

We do not use the term relationship elsewhere very much. In the early days of Carnival, we adopted the semantic terms more thoroughly, entity and relationship rather than vertex and edge. Since then, we have moved towards vertex and edge, which will be more understandable to programmers. Probably best to move to EdgeDefinition.

Break individual classes out of GraphSchema.groovy

It contains many classes, VertexLabelDefinition, VertexPropertyDefinition, etc. Break them out into individual files so they are more visible in the IDE.

Vertex, Property, and Edge DefTrait rename

The fact that these things are named with Trait makes the code less readable. There is code like:

void someMethod(VertexDefTrait vDef) { }

VertexDefTrait vDef =

It's weird. Yes they are traits, but it would be more clear and readable if we had:

void someMethod(VertexDefinition vDef) { }

VertexDefinition vDef =

Consider a rename. The issue will be that we already have several names in this space. VertexInstanceDefinition should go as per below. VertexDefinition is claimed by the annotation. Perhaps we should rename the @VertexDefinition to @VertexModel. That would probably be more clear. Then VertexDefTrait could be named VertexDefinition.

Keep/remove VertexInstanceDefinition?

VertexInstanceDefinition looks like an alternative template mechanism for singleton vertices to VertexBuilder. I think it is a hold-over from the historical object based ControlledInstance code. I do not think we need it any more. It should be removed for clarity.

Duplicate class name - ElementDef

There is an ElementDef in carnival.graph and carnival.core.graph. The subclasses of ElementDef, VertexDef and EdgeDef, are very thin. There is only one method, lookup(). We should probably put all three methods into a single class ElementDefinition.

carnival-data / carnival

GraphSchema cleanup #98