horizonclient: next and prev methods #1278
Conversation
poliha
added
the
horizonclient
| @@ -539,5 +539,29 @@ func (c *Client) PrevAssetsPage(page hProtocol.AssetsPage) (assets hProtocol.Ass | |||
| return | |||
| } | |||
|
|
|||
| // NextLedgersPage returns the next page of ledgers | |||
| func (c *Client) NextLedgersPage(page hProtocol.LedgersPage) (ledgers hProtocol.LedgersPage, err error) { | |||
| err = c.sendRequestURL(page.Links.Next.Href, "get", &ledgers) | |||
There was a problem hiding this comment.
Since these are simple and repetitive I think you could just inline all these next/prev returns, and lose the err variable.
clients/horizonclient/client.go
Outdated
| @@ -539,5 +539,29 @@ func (c *Client) PrevAssetsPage(page hProtocol.AssetsPage) (assets hProtocol.Ass | |||
| return | |||
| } | |||
|
|
|||
| // NextLedgersPage returns the next page of ledgers | |||
There was a problem hiding this comment.
Add a period to the end of all the godoc strings here.
| fmt.Println(err) | ||
| return | ||
| } | ||
| fmt.Print(efp) |
There was a problem hiding this comment.
should this be ftm.Println?
There was a problem hiding this comment.
Any will do actually. Oped for fmt.Print here because the response will not be a simple string.
| fmt.Print(efp) | ||
|
|
||
| // next page | ||
| nextPage, err := client.NextEffectsPage(efp) |
There was a problem hiding this comment.
Might be cool to have the example use a loop, to show calling NextEffectsPage repeatedly. How do we know when we reach the end? Would be good to show that too.
There was a problem hiding this comment.
Yes that's a good point. will need to confirm what the indicates the end of a collection of pages here.
| fmt.Println(nextPage) | ||
| } | ||
|
|
||
| func ExampleClient_PrevEffectsPage() { |
There was a problem hiding this comment.
Same comments as for previous example
| @@ -199,5 +199,29 @@ func (m *MockClient) PrevAssetsPage(page hProtocol.AssetsPage) (hProtocol.Assets | |||
| return a.Get(0).(hProtocol.AssetsPage), a.Error(1) | |||
| } | |||
|
|
|||
| // NextLedgersPage is a mocking method | |||
There was a problem hiding this comment.
What happens when there is no further Next/Prev to return? Can this be tested?
There was a problem hiding this comment.
Will look into this.
This is part of a set of PRs to add methods to get the previous and next pages of a response from horizon.
This PR implements these methods for the ledgers and effects page.
Other pages will be implemented as listed in #985