a minor issue with coding style, doesn't affect correctness

[vladimir-range]

Hi Jan, sorry if this interferes with your other priorities, I wasn't sure whether I should report it, if you just close it, it's fine.

I only inspected a part of Bybit.Net and Coinbase.Net so maybe it's in other places and projects too. I have noticed a couple of things in GetKlinesAsync():

1. var minOpenTime = result.Data.Min(...);
   bybit returns records in descending order, so there is really no need to calculate minOpenTime - it's in the last record; if you don't trust the input then maybe more validation is needed, like verifying that the records are at the multiples of interval from each other and may be even verifying that a kline starts where the previous one stopped; otherwise it unnecessarily (though ever so slightly) increases the carbon footprint; for coinbase the order seems to be reversed judging by result.Data.Reverse(), so may be the uniform minOpenTime calculation is just to save your time, then it's totally fine.

2. return result.AsExchangeResult<IEnumerable>...Select(...).ToArray()
   Select() is implemented to be executed lazily, ToArray() makes it to evaluate eagerly; if it's done consistently across all libraries, then may be there is no point to pretend that a more abstract type is returned.

the respective files are:
Coinbase.Net\Coinbase.Net\Clients\AdvancedTradeApi\CoinbaseRestClientAdvancedTradeApiShared.cs and Bybit.Net\ByBit.Net\Clients\V5\BybitRestClientApiShared.cs

[JKorf]

Hi,

Thanks for your input, I appreciate it.

1. As you already mentioned this was generally done in a similar way in all libraries to save some time. A lot of these endpoints and implementations are very similar so it was just copied from one, pasted in the other and adjusted slightly to fit the API. It's a valid point that the call is not needed, all exchanges consistantly return data in a specific order so it should be enough to take either the last or first element. That being said, I might get around to it at some point but it's not a very high priority.

2. Yes I'm aware of the workings of Select and you're right that it doesn't make much sense here. This is actually coming from something I still want to fix. All lists in response models from API calls currently have the IEnumerable<> type, while in practice it makes more sense that these are array types. Array types also prevents the user having to often do a ToList or ToArray themselfs, which is unnecessary as the deserialized type is already a concrete type.
   The only positive about returning IEnumerable types is that the json converter can decide what the best concrete type would be, and when there is only a single entry in the array it will create a specific readonly single-item type (forgot what it's called) which I'm sure has some small performance benefits.
   Coming back to the original point, since everything already returns IEnumerable types in the library I made the shared interfaces the same.
   The issue with changing all the IEnumerable types to concrete types is that it's a lot of work, and a breaking change for all parts of each library.

[vladimir-range]

I forgot to say how impressed I was with the scope, design and implementations of your projects. Best wishes.