bchavez
commented
7 years ago Hi @epsitec ,
I think your suggestion makes sense. However, I think breaking the driver into too many pieces can make things initially harder to discover and use for developers. It's been my personal experience with some NuGet packages where project X is split into 5-10 separate packages, and I'd have to go digging through setup/install documents trying to figure out what packages I need; just to get something simple working. So, I'd like to avoid this particular WTF experience as much as possible.
With that said, though, moving the AST into a separate assembly, _could_ be seen as beneficial. Especially for something like Issue #92 where some runtimes don't have a full networking stack support. It could make the driver's AST component easier to consume in these special case runtimes. Although, cross-platform support for .NET Core has been expanding for these specialized runtimes, so consuming the driver on these specialized runtimes may not be an issue in the not-too-distant future.
My hunch is, in the vast majority of cases, projects consuming the RethinkDb.Driver most likely use both the AST and networking stack together hand-in-hand so one could make the argument that it's best to keep them packaged together.
Furthermore, before we consider breaking up the driver, I do think we have more pressing issues to attend to; like #77, #96, and #103. In particular, after my initial refactoring for #77, I was able to get a better architectural separation of concerns for the AST and JSON emitting/parsing codes. The initial #77 refactoring, from what I saw, made #92 and #104 (this issue) more easily achievable.
Is there any practical reason why the networking references are a problem in your project? Or are you approaching this purely as reference optimization & architectural correctness problem?
Many thanks, Brian
:watch: :citysunset: [**"I just can't wait... I just can't wait... for saturday night..."**_](https://www.youtube.com/watch?v=wOjGmpmvGz8#t=2m)
epsitec
The current implementation provides a monolithic driver where the AST and the communication with the server are in the same module.
I feel that the code responsible for talking to the server should not be bundled with the AST code (and maybe other separations should be added too, but I've not digged into the various concerns to produce helpful ideas here).
In our software, we have a general querying layer which produces ReQL expressions. These are then serialized and passed to a querying engine, which runs them agains the RethinkDB server. In this scenario, the general querying layer needs to include the driver as a dependency in order to create the ReQL expressions. And this looks like a code smell. Only the querying engine, which needs to talk the the server, should require the driver. The higher level abstraction should be able to only use the AST portion of the library.
What do you think? Am I overly concerned ;-) about this separation of concerns?