Giter Site home page Giter Site logo

Comments (7)

rossmills99 avatar rossmills99 commented on June 24, 2024

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

from arangodb-net-standard.

DiscoPYF avatar DiscoPYF commented on June 24, 2024

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?

from arangodb-net-standard.

rossmills99 avatar rossmills99 commented on June 24, 2024

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.

from arangodb-net-standard.

apawsey avatar apawsey commented on June 24, 2024

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.

from arangodb-net-standard.

rossmills99 avatar rossmills99 commented on June 24, 2024

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..

from arangodb-net-standard.

apawsey avatar apawsey commented on June 24, 2024

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?

from arangodb-net-standard.

rossmills99 avatar rossmills99 commented on June 24, 2024

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?

from arangodb-net-standard.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.