Consider changing the naming used for data model carrying parameters

[DiscoPYF]

I got confused by the naming of the data models that carry parameters for the client methods. I didn't realize at first the distinction between the models for body parameters and the other for query parameters.

Example: PostCollectionRequest (JSON body parameters) and PostCollectionOptions (query parameters)

What about renaming them to make that distinction more obvious?

Example: PostCollectionBodyParams and PostCollectionQueryParams?

I find using the term Options especially confusing.

Maybe when using the client in code the distinction is more natural. I made this consideration while looking at the classes rather than actually using one of the client.

[rossmills99]

I see your point. What do you think of PostCollectionBody and PostCollectionQuery, just to make things a bit more compact?

[DiscoPYF]

Nice, to me it's more obvious and the naming makes sense when you consider that the project is simply a wrapper over the HTTP api. I would be worried that PostCollectionQuery can be confused with something related to an ArangoDB "query", but it should be fine in the context where those class are used.
@AlistairB99124 Any thoughts?

[rossmills99]

So from our offline conversation we agreed to change to use Body and Query as suffixes for the POST body model and query params models. Let's use this issue to change existing models and also make sure future models get these standard suffixes.

[apawsey]

Following from the point raised in... #201

• Renaming the "query" objects to "options"

I'll be honest I didn't go back through all of your issues, so I hadn't seen the previous discussion. I see the point that was raised, and it's not exactly a big issue, however I would disagree with the conclusions. Firstly the point of an interface like this is to abstract away from the API. The API is inconsistent and somewhat of a pain, as I realised running all your unit tests (good job on those by the way!). So as far as the user (of this API) is concerned, it shouldn't matter to them how the "options" are passed to Arango. Perhaps tomorrow they change the "options" to be passed as headers instead of the querystring? Do you want to then have to go through and rename all the "Query" objects to "Headers"? Secondly, the things we are talking about here are specifically "options". They don't affect what is done, they only affect how it is done. If I set or don't set the "WaitForSync" option, in principle it makes no difference to the end result (yes there is the potential for it to raise a different result if the disk sync failed for some reason), but in principle it is just an option as to how the work is performed. The items in the body models are the parameters, which affect what is done, and by the same principle as the Querystring/Headers example above, I would also suggest that they should lose the "Body" suffixes.

[rossmills99]

Hi, thanks for spending the time to give your point of view here!

the point of an interface like this is to abstract away from the API

I think that's true but early on we took the decision that we did not want to put too much abstraction between users of the API and the ArangoDB REST API itself. This came from an observation that the abstraction often obscures what is actually going on, which can be unhelpful. So we chose to retain names that reflect the underlying REST API rather than obscure it. I suppose the aim is that users who understand the ArangoDB REST API documentation can immediately understand know how to use this API client.

All that being said, I agree that there is definitely a place for a more abstracted view. In our case we might build that layer in our application code, using this client under the hood.

Perhaps tomorrow they change the "options" to be passed as headers instead of the querystring

I see your point although that example seems very unlikely. But there are lots of ways ArangoDB could make breaking changes in their REST API where we would have to change our API to accommodate it. Renaming *Query classes to *Options instead wouldn't save us from most of them.

It would also break the "consistency" aspect I mentioned above. If an "Options" object might contain a mix of values then someone who knows the ArangoDB REST API might have difficultly understanding how the values are applied in our API. In addition I think it would make maintaining the API slightly more complex (contributors have to understand how different options will be applied for different options models, meaning they have to read code they otherwise could have made safe assumptions about).

Hopefully this gives you more insight into our thinking here..

[apawsey]

I do follow your thinking, but I still respectfully disagree.

Entity Framework could be described (partly and I know at a stretch) as a wrapper around SQL statements, but it doesn't make considerations that it might confuse people who know SQL.

I would still strongly argue the purpose of an API like this library is to take an interface and re-present it in a way that better suits the paradigm or environment that it's being used in. REST API's are great for presenting a normally idempotent, fairly fault tolerant, and reasonable simple, access point into a system. They don't fit very well however with object orientated code.

I understand the idea of keeping familiarity with the Arango API, but I still argue that that isn't the point. To me, what you're essentially saying is I need to understand your API and the Arango API, to successfully use this library. I need to understand how you've arranged the client classes, and how the serialization is performed, but I also need to understand the HTTP API's parameters etc. As a user, if I'm familiar with the Arango API, why would I need this library? Most project's won't use all the different API's you've provided, so I might as well just write out code to handle the 8 methods my project uses.

As a user not familar with the Arango API, this library is perfect. I download, put in an hostname, and away I go. That's surely the objective here?

[rossmills99]

I have to say you make a good argument. This is exactly the sort of thing I could internally debate with myself and I'm not sure if I would ever be able to decide what is truly the best approach. I see both sides of the argument. But let's also remember we are just talking about the suffix used in names of certain models in the API. I think most of us are able to handle either naming convention without blinking. That might actually be the deciding factor in favour of the status quo - changing it doesn't seem to bring any real benefit. If the HTTP convention called this the "options string" instead of "query string", what would we really be debating here?