-
-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Break up the driver into smaller pieces #104
Comments
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 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 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, ⌚ 🌆 _"I just can't wait... I just can't wait... for saturday night..."_ |
There is no technical reason for this separation. Referencing the driver assembly from our general querying layer is working just fine. It is more an architectural concern for now. |
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?
The text was updated successfully, but these errors were encountered: