WIP drop key based traits (e.g. HasName, HasVersion etc.)

[mpollmeier]

looks like they're not used anywhere, and they're sort of in the way of
a refactoring I'd like to do

[fabsx00]

I think I've seen them in use, but II'm also in favor of dropping them. One less thing to know about.

[ml86]

Those are used in codescience. Search for "nodes.Has" and you will get ~15 findings.
I guess we could get along without those traits... But this would mean match... in all those places.

[fabsx00]

Hm, ok... then let's rather keep them.

[mpollmeier]

Hmm, I thought I had checked that locally, but you're right they're indeed used there. Thanks for catching @ml86!
I'm having a look if we really need them and otherwise come up with an alternative.

[ml86]

So why do you want to get rid of the property traits @mpollmeier ?

[mpollmeier]

Ok, some context for this change: our schema DSL has always defined the cardinality on the property, i.e.

val exampleProperty = builder.addProperty(
  name = "example", 
  valueType = ValueTypes.STRING, 
  cardinality = Cardinality.ZeroOrOne,
  comment = "lorem ipsum")

fooNode.addProperty(exampleProperty)

IMO it would be better to specify the cardinality when adding it to a node, and use a sensible default (maybe Cardinality.ZeroOrOne?), e.g.:

val exampleProperty = builder.addProperty(
  name = "example", 
  valueType = ValueTypes.STRING, 
  comment = "lorem ipsum")

fooNode.addProperty(exampleProperty)

// or the equivalent of Cardinality.One
fooNode.addProperty(exampleProperty.mandatory)

The change above impacts the way we define the trait Mixins, because they take into account the cardinality. In the first example, we'd generate a trait WithExampleProperty { def example: Option[String] }.

My initial thought was to just drop those traits, but given that you'd prefer to keep them, we could alternatively generate trait WithExampleProperty and trait WithExamplePropertyMaybe, in case both variants are used somewhere. The downside is that we now have two static types that we need to check.

A third option would be to change all these property based mixins to Option types, i.e. even when we statically know that e.g. FooNode.example is mandatory, the mixin would be trait WithExampleProperty { def example: Option[String] }.

[bbrehm]

I don't see the point in all the Option stuff. Is there a specific property you have in mind that has different cardinalities on different nodes?

Is there a specific example where the JVM default (null for reftypes, zero-bytes for valtypes) is inacceptable?

[mpollmeier]

The point of Option is to statically know if a property is mandatory or optional.
The JVM defaults do not allow to differentiate that - let's stick to reftypes to keep it simple: they're always optional since you can always set them to null.

Why do we want to know that statically? To guide the user who's writing queries/code using our API. Tell them that a value may or may not be there.

Java didn't use to have that and developers had to add null-checks everywhere, because everything may be null. What a terrible way to write code. If they forgot (happens easily) they'd run into a NPE at runtime.

That's why java.util.Optional was added in Java 8.

[bbrehm]

Let me repeat the question:

Is there a specific property you have in mind that has different cardinalities on different nodes?

[mpollmeier]

I have seen instances where a property was used like a mandatory property based on the knowledge "if it's node type X, it'll be set". No, I can't find it right now, but that's not even the point.

The point is: the current schema DSL doesn't make it easy to make the differentiation of mandatory/optional, but IMO it should be easy. Because it's not, we're driving users of the DSL (incl. ourselves) into making bad decisions, e.g. to using the same property and encoding it's cardinality when using it, e.g. by calling .get on it in the main codebase "because we know".
This is bad and the suggested refactoring would fix the issue.

And you'd even keep the mixin traits. So let's turn it around: what's the problem with this suggestion, how does it make your life harder?