neo4j / neo4j-dotnet-driver

Neo4j Bolt driver for .NET
Apache License 2.0
232 stars 69 forks source link

Driver execute API #651

Closed thelonelyvulpes closed 1 year ago

thelonelyvulpes commented 2 years ago

e.g. BookmarkManager in QueryConfig - //Default is the drivers non mutable driver level bookmark manager. Can be set to null to disable causal chaining.

This is a differentiation of behaviour driven by only implicit or explicit null. I think this is a risk of confusion. As defaults are required to be compile time constants we can't defualt to using specific bookmark manager(unless we overload the constructor or use the builder patteen) for one that takes a bookmark manager and one that doesn't.

AndyHeap-NeoTech commented 2 years ago

e.g. BookmarkManager in QueryConfig - //Default is the drivers non mutable driver level bookmark manager. Can be set to null to disable causal chaining.

This is a differentiation of behaviour driven by only implicit or explicit null. I think this is a risk of confusion. As defaults are required to be compile time constants we can't defualt to using specific bookmark manager(unless we overload the constructor or use the builder patteen) for one that takes a bookmark manager and one that doesn't.

I was under the impression that a default was part of the ADR so that ExecuteQuery has causal chaining out of the box, is that not the case?

thelonelyvulpes commented 2 years ago

e.g. BookmarkManager in QueryConfig - //Default is the drivers non mutable driver level bookmark manager. Can be set to null to disable causal chaining.

This is a differentiation of behaviour driven by only implicit or explicit null. I think this is a risk of confusion. As defaults are required to be compile time constants we can't defualt to using specific bookmark manager(unless we overload the constructor or use the builder patteen) for one that takes a bookmark manager and one that doesn't.

I was under the impression that a default was part of the ADR so that ExecuteQuery has causal chaining out of the box, is that not the case?

It currently does default to have causal chaining, we have no way currently of disabling causal chaining other than passing a NoOpBookmarkManager...

RichardIrons-neo4j commented 2 years ago

e.g. BookmarkManager in QueryConfig - //Default is the drivers non mutable driver level bookmark manager. Can be set to null to disable causal chaining.

This is a differentiation of behaviour driven by only implicit or explicit null. I think this is a risk of confusion. As defaults are required to be compile time constants we can't defualt to using specific bookmark manager(unless we overload the constructor or use the builder patteen) for one that takes a bookmark manager and one that doesn't.

I was under the impression that a default was part of the ADR so that ExecuteQuery has causal chaining out of the box, is that not the case?

It currently does default to have causal chaining, we have no way currently of disabling causal chaining other than passing a NoOpBookmarkManager...

I think it'd be nice to have some static methods here so that it's always clear what you're doing, like

manager = BookmarkManger.CreateOrWhatever(config)
// or
manager = BookmarkManager.DefaultBookmarkManager()
// or
manager = BookmarkManager.NoOpBookmarkManager()

and those are the only ways you can create a bookmark manager, and you can't pass null.