This repository has been archived by the owner on Feb 9, 2018. It is now read-only.
forked from VojtechVitek/go-trello
-
-
Notifications
You must be signed in to change notification settings - Fork 2
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
2 changed files
with
81 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bf56605
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you think this will make the code base unnecessarily huge? I mean, if we are going to write a new struct and validation method for
AddCard
we are easily going to write custom structs and validation methods for almost all Trello API calls. Wouldn't this be a mess?bf56605
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this is a stupid idea, but I can't stop thinking we should just accept a
map[string]interface{}
everywhere, serialize it to JSON and make the request with it. Wouldn't it be an easy solution?(Instead of VojtechVitek#24, for example)
bf56605
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd strongly object against using
map[string]interface{}
as you then need to to type conversions all the time and need to handle the types yourself. You're loosing the advantage of a strongly typed language like Go.Having input-structs for functions requiring a lot of parameters which are then passed to the API is a standard I see all over Go libraries (for example libraries by Amazon, Google and other widely used libraries) so I tend to follow their example when implementing this. You know what to pass in and what type to pass in when you're building your application around the library instead of passing a generic map not telling you which parameters are there, which are required and what type they are…
bf56605
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I understand the point.
map[string]interface{}
wasn't a real well-thought suggestion, it was there just to compose the argument of the my first comment.I understand that having proper well-defined types makes the library better and is the standard -- and it really should be! --, but perhaps considering that this is not much of a library, but more of a thin wrapper around the Trello API, and considering that people using this will surely need to know about the Trello API (for example, the documentation for the methos of this library don't explain anything, it just shows links to Trello API documentation), perhaps we don't need all this (again, considering the amount of work and hugeness of the code base that well-defined types will require), perhaps, then
map[string]string
(you'll object to this also, I think, but hey, we don't even need to check the existence of keys or anything like it, just serialize to JSON)?Converting a
map[string]string
to JSON is worse than converting a struct, I know, but I'm talking about a trade-off here. Does that make sense? Am I totally wrong in thinking this way?bf56605
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why serializing to JSON? The API does not accept JSON but only
application/x-www-form-urlencoded
… I don't see any advantage of a map against a well written library / wrapper… Sure, you might spare some minutes not writing a struct with defined parameters but imho that's only technical dept… You will need to keep the documentation open all the time you're using the library looking up which parameters you can pass in. With a well defined interface you have the property names to see what you can pass in and what type the value has. (Sure with changing types likefloat32
andstring
for position that's hard too but thats the implementation of the API which we can't change)