Leena #11
Leena #11
Conversation
| @@ -9,3 +9,27 @@ require ( | |||
| github.com/joho/godotenv v1.4.0 | |||
| google.golang.org/api v0.60.0 | |||
| ) | |||
|
|
|||
There was a problem hiding this comment.
not sure i was meant to change this
There was a problem hiding this comment.
The same thing happened to me here probably @tomw-1 knows better what's happening
| ) | ||
|
|
||
| func NewRouter() *martini.ClassicMartini { | ||
| libraryController := new(controllers.LibraryController) | ||
|
|
||
| router := martini.Classic() | ||
|
|
||
| router.Get("/books", libraryController.ListAll) | ||
| router.Get("/books/:id", libraryController.GetByKey) | ||
| router.Get("/books", initBookInterface, libraryController.ListAll, closeClientConnection) |
There was a problem hiding this comment.
👍 I like the way you organised the routes
| query := datastore.NewQuery("Book").Order("Id") | ||
|
|
||
| if len(title) > 0 { | ||
| query = query.Filter("Title =", title) |
There was a problem hiding this comment.
how would you change this to allow the filtering by title and author? @tomw-1 asked me the same thing 😄
There was a problem hiding this comment.
add another Filter for author
| return book, nil | ||
| } | ||
|
|
||
| func (bookImpl BookImplementation) Query(title string) ([]Book, error) { |
There was a problem hiding this comment.
I would probably have put this in your helper class, I have a feeling that's more appropriate for it to live there. What do you think about it?
There was a problem hiding this comment.
actually I was very close to just extracting all the functions in a datastore file and expose required functions to all models, as with addition of new entities these functions could easily be reused, I might do if i get chance on Wednesday
| } | ||
|
|
||
| func NewBookImplementation() BookInterface { | ||
| ctx := context.Background() |
There was a problem hiding this comment.
genuine question. because I still have to dig deeper into context, what was your feeling of moving the context from the controller to the model itself? And what happens if you need to create another controller and anothe model? 😄
There was a problem hiding this comment.
I haven't used context a lot myself but In our example context don't hold any info which is why at that time I just got lazy and left it there. I would have probably initialised context on server initialisation and passed in to controllers where it could hold info on request timeouts etc and is also used beyond model.
| if _, err := client.PutMulti(ctx, keys, books); err != nil { | ||
|
|
||
| bookInt := models.NewBookImplementation() | ||
| if err := bookInt.PutMulti(books); err != nil { |
| @@ -9,3 +9,27 @@ require ( | |||
| github.com/joho/godotenv v1.4.0 | |||
| google.golang.org/api v0.60.0 | |||
| ) | |||
|
|
|||
There was a problem hiding this comment.
The same thing happened to me here probably @tomw-1 knows better what's happening
| return books, nil | ||
| } | ||
|
|
||
| func (bookImpl BookImplementation) Put(book Book, insert bool) (int64, error) { |
There was a problem hiding this comment.
the code is perfectly fine, what do you think about having two separate functions that for create and update without the need of passing a boolean?
There was a problem hiding this comment.
yeah I could for better readability, there I was just trying to not duplicate code
| "net/http" | ||
| ) | ||
|
|
||
| func WriteJsonResp(w http.ResponseWriter, data interface{}) { |
There was a problem hiding this comment.
🥳 nice one, I like the two functions to manage errors and responses
No description provided.