knowm / XChange

XChange is a Java library providing a streamlined API for interacting with 60+ Bitcoin and Altcoin exchanges providing a consistent interface for trading and accessing market data.
http://knowm.org/open-source/xchange/
MIT License
3.86k stars 1.94k forks source link

About OpenOrderParams,TradeHistoryParams and Params classes in general #2411

Open makarid opened 6 years ago

makarid commented 6 years ago

Hello, instead of having many Params interface classes ,is it better to create only one ParamsAll class for every case? TradeHistoryParamsAll already exist, why don't we delete all the other TradeHistoryParam classes? For CancelOrderParams,why don't we create a CancelOrderParamsAll class to include every parameter that any exchange may ask? We can do the same for OpenOrderParams. Thanks

timmolter commented 6 years ago

Someone else can probably answer this better than me, but I believe it's probably because mostly of the time a certain type of params is required, and using instance of it can be verified and if someone uses the wrong type an exception will be thrown with a very clear message on what needs to be changed.

  @Override
  public List<Ticker> getTickers(Params params) throws IOException {
    if (!(params instanceof CurrencyPairsParam)) {
      throw new IllegalArgumentException("Params must be instance of CurrencyPairsParam");
    }
    Collection<CurrencyPair> pairs = ((CurrencyPairsParam) params).getCurrencyPairs();
    CurrencyPair[] pair = pairs.toArray(new CurrencyPair[pairs.size()]);
    return KrakenAdapters.adaptTickers(getKrakenTickers(pair));
  }

I'm sure that this could be cleaned up though and made more elegant. What do you propose?

makarid commented 6 years ago

When i am using tradehistoryparamsall and i set for example only currencypair,if the exchange needs more info in order to return tradehistory an exception occures which says where exactly is the problem.So i don't think that this is the case. We could create a class like tradehistoryparamsall for cancelOrders,openOrders and anyother that needs many different params in order to return something from the exchange.

elkceo commented 3 years ago

When i am using tradehistoryparamsall and i set for example only currencypair,if the exchange needs more info in order to return tradehistory an exception occures which says where exactly is the problem.So i don't think that this is the case. We could create a class like tradehistoryparamsall for cancelOrders,openOrders and anyother that needs many different params in order to return something from the exchange.

Is that was realized? How I can get these TradeHistoryParams?? FTX exchange