search

bitfinexcom / bitfinex-api-go

BITFINEX Go trading API - Bitcoin, Litecoin, and Ether exchange
https://www.bitfinex.com/
MIT License
303 stars 222 forks source link

What is the point of the various *Snapshot types? #194

Open abrander opened 4 years ago

abrander commented 4 years ago

While evaluating the v2 API I was wondering why many functions have a signature like

type SomeServiceAllSnapshot {
    Snapshot []TheRealType
}

func (fs *SomeService) All() (*bitfinex.SomeServiceAllSnapshot, error) {

This creates some awkwardness. First of all, it just feels weird to have the result wrapped in a Snapshot type.

As it's implemented now, the caller cannot assume that the first value returned is non-nil even if the error is nil. This will require the API user to write code like 

something, err := client.SomeService.All()
if err != nil {
    // handle error
}

if something == nil {
    // handle an empty snapshot
}

for s := range something.Snapshot {

If the functions had a signature like:

func (fs *SomeService) All() ([]bitfinex.TheRealType, error) {

It could be used more idiomatic like this:

all, err := client.SomeService.All()
if err != nil {
    // handle error
}

for s := range all {

1) Is there a reason that the Snapshot types are exposed for the end user? 2) Would you be interested in a pull request changing the API?

Issue type