OrderBook asks and bids sorting

[cymp]

Hi,

Does anyone know a good reason why we don't sort lists in the order book constructor?

public OrderBook(Date timeStamp, List asks, List bids) { this.timeStamp = timeStamp; this.asks = asks; this.bids = bids; }

IMO, and since the comparator of LimitOrder is correctly implemented, they should always be sorted, something like:

public OrderBook(Date timeStamp, List asks, List bids) { this.timeStamp = timeStamp; this.asks = new ArrayList<>(asks); this.bids = new ArrayList<>(bids); Collections.sort(this.asks); Collections.sort(this.bids); }

This would prevent from having unexpected OrderBooks if some exchanges decide not to return the orders list in the usual expected order book order which is what happens here sometimes: https://www.okex.com/api/v1/depth.do?symbol=tct_btc&size=2

[rafalkrupinski]

Well, in that case the exchange specific client code should do it. Why waste cpu time sorting already sorted collections for all the other cases?

[timmolter]

If we sort in the generic API layer then unnecessary sorts will always be carried out, but the ALL orderbooks will be nicely sorted. This is how the Trades class is implemented. It does make more sense to sort at the raw layer though only for those exchanges that don't return the data sorted.

[cymp]

@rafalkrupinski @timmolter Yeah these are precisely the reasons why I didn't want to make a PR for this code at the OrderBook level, and why I already did this at the raw layer. But on the other hand, I am not satisfied by the contract offered by the OrderBook constructor, especially in this case where it is easy to create an order book inconsistency. Moreover, as you may know, the exchanges APIs can return data sometimes not consistent for no reason, or can change...

Something like Guava's TreeMultiset instead of Lists would have been preferable, but I cant' find any equivalent in the standard Java API.

Well unless anyone can find a better solution, I will continue sorting at the raw layer level.

[rafalkrupinski]

You might add something like this

 assert assertSorted(asks)
 assert assertSorted(bids)

boolean assertSorted(List orders){
  List copy = new ArrayList(orders);
  Collections.sort(copy);
  return copy.equals(orders);
}

Then you can run your code with -ea when you wan't to be extra sure that your exchanges return sorted data.

[cymp]

I have created a PR providing the OrderBook constructor allowing to sort if someone specifically want to have the data sorted though. https://github.com/timmolter/XChange/pull/2378.

[cymp]

@rafalkrupinski Sorry, made the PR before reading your answer, I will instead use my constructors.