Skip to content
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

Consider switching to standard .NET naming/code conventions. #18

Closed
DarrellBailey opened this issue Jan 14, 2016 · 5 comments
Closed

Consider switching to standard .NET naming/code conventions. #18

DarrellBailey opened this issue Jan 14, 2016 · 5 comments

Comments

@DarrellBailey
Copy link

Is there any particular reason the standard java naming conventions were maintained in the port? Was this for consistency or sameness with the java driver?

In some places the driver seems to embrace some c# features, such as with async/await. However, in other places the driver still feels extremely Java-like, such as the use of getter and setter methods instead of properties.

Is documentation available on where the line in the sand is being drawn for what is being altered in the port and what is not?

@bchavez
Copy link
Owner

bchavez commented Jan 14, 2016

Hi Darrell,

Thanks for your question.

You're exactly right. The driver almost feels like it has a split stylistic personality. I'll explain the reasons behind it.

There is a proverbial line in the sand and it exists between:

  1. The public ReQL API / AST when composing and running a ReQL Query follows the official Java driver style.
  2. The supporting infrastructure around the ReQL AST follows .NET/C# style. For example, class names for infrastructure components, connections, sockets, exceptions, supporting models, connection pooling, and their implementations follow .NET/C# conventions. Here, the architecture is similar to Java but Java naming conventions were not carried over.

Regarding No.1

First, the ReQL AST / Query Terms (mostly everything inside Source/RethinkDb.Driver/Generated/Ast) follows naming conventions that have been carried from the Java port. Mostly all the methods defined on the AST are Java style (ex: r.now() instead of R.Now()). The primary reason for this is API compatibility. Originally, I made an attempt to PascalCase the AST but quickly accumulated a lot of extra maintenance code to handle the transition from Java -> C#. There were just too many edge cases to handle so I backed out of the PascalCase idea quickly. This was especially problematic when porting the automated YAML test suite with over 1000+ unit tests. Also, PascalCasing deviated from the official API of all 4 official drivers.

I also thought about maintaining a metacsharp.py python script similar to how the Java driver generates the Java AST terms and YAML tests but it would have required me to maintain the entirety of the python tooling anytime metajava.py (and associated test scripts) changed. It would have also require an installation of python on the build machine. All this was a bit too much. I wanted to keep everything low maintenance and in the C# language as much as possible.

The win here is, anytime the RethinkDB team changes the Java driver. It's relatively easy to just pull down some intermediate .json files, plug them into the C# driver and regenerate the AST and YAML tests. Additionally, (and a huge win) is we get free ReQL documentation (complements of the Java driver). The ReQL virtually translates 1-to-1 between Java and C#.

Regarding No.2

Second, the supporting infrastructure around ReQL, like connections, all have implementations _similar to_ but different enough from Java. These implementations aren't public, so idiomatic .NET/C# was used. Additionally, the Java driver currently doesn't support multi-threading while the C# driver does. I felt it was really important for the C# driver to support multi-threading out of the box. This driver would most likely be used in server applications like ASP.NET where multiple threads can hit the driver at any given time. Hence, the async/await. You'll also notice the run methods like conn.runAsync Java style feel because the method call is still part of the public API for ReQL.

Summary

Declaring a type reference to an IConnection feels like .NET (prefixed I) but actually composing (and running) a ReQL query feels like all the other official drivers: Java, python, and javascript. Roughly any method exposed by the public ReQL API documentation follows official naming of the Java driver.

If you have any other thoughts or concerns, please feel free to comment. Or, if I've answered your questions we can close the issue.

Thanks,
Brian

@bchavez
Copy link
Owner

bchavez commented Jan 14, 2016

Also, you mentioned there were some Java getter and setter methods. Which getter/setter methods are you referring to?

Documentation for the internal changes can be found here:

@bchavez
Copy link
Owner

bchavez commented Jan 19, 2016

Hi @DarrellBailey,

It's been a few days, how is it going? Still using the driver? Do you still feel strongly about the naming convention of the ReQL API? Do you have anymore thoughts?

If not, I'd like to close the issue in a few days.

@bchavez bchavez closed this as completed Jan 20, 2016
@DarrellBailey
Copy link
Author

No you answered my question fully and succinctly. Forgive my late response.

This is good stuff! Thanks for all your work!

@bchavez
Copy link
Owner

bchavez commented Feb 13, 2016

Hi @DarrellBailey , so we finally moved to .NET naming conventions for the public ReQL AST and public Connection APIs. Pretty much, anything public should be following good o'le .NET naming conventions.

Here's a quick example:

public static RethinkDB R = RethinkDB.R;
...
var poco = new Foo { Bar = 1, Baz = 2};
var result = R.Db("mydb").Table("mytable").Insert(poco).Run(conn);

Going forward, anything after v2.2.5-beta-3 should be using the new conventions.

PS. Anonymous Type Optional Arguments still use new{ index = "foo_ix"}.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants