lh-api #4
lh-api #4
Conversation
Redis repo, first CRUD operations
Co-authored-by: Luke Rogerson <[email protected]>
service/get_amount_out_test.go
Outdated
| newOrder(1001, 2), | ||
| newOrder(1002, 3), | ||
| }, | ||
| index: -1, |
There was a problem hiding this comment.
Shouldn't index start at 0?
There was a problem hiding this comment.
no as first action is promotion
| } | ||
| // create order iter | ||
| return &OrderIter{ | ||
| index: -1, |
There was a problem hiding this comment.
Why initialise this with -1? Better to start with 0 so it aligns with how we usually think about array indexes?
There was a problem hiding this comment.
i like the use of -1 as in "not been used" rather than 0 which is more implicit in case of only one order in the array
data/redisrepo/order_iter.go
Outdated
| } | ||
|
|
||
| func (i *OrderIter) Next() *models.Order { | ||
| ctx := context.Background() |
There was a problem hiding this comment.
We can leave this for now, but this context should be from a function argument (eg. Next(ctx context.Context) so that we're using the same context all the way through the app
| return &OrderIter{ | ||
| index: -1, | ||
| ids: orderIDs, | ||
| redis: r, |
There was a problem hiding this comment.
I don't think you are using this redis property anywhere? Can it be removed?
There was a problem hiding this comment.
taking it back, ofcourse it is used in t he redis impl
service/lh.go
Outdated
| @@ -0,0 +1,95 @@ | |||
| package service | |||
There was a problem hiding this comment.
Please add logs to the methods below
|
Left some feedback |
No description provided.