CryptoKraken / crypto-kraken-services

MIT License
0 stars 1 forks source link

It's needs to change the signature of the deleteOrder and getOrderInfo methods #82

Closed skubarenko closed 6 years ago

skubarenko commented 6 years ago

This method should get an instance of the Identified<Order> instead of an order id. Because some services require an extra information about an order, not only its id. For example, the KuCoin service requires the symbol parameter which can be only obtained from the instance of the order type.

This is also applies to the getOrderInfo method. This method requires the symbol and type parameters in the KuCoin service.

async deleteOrder(identifiedOrder: Identified<Order>, exchangeCredentials: ExchangeCredentials): Promise<boolean>;
async getOrderInfo(identifiedOrder: Identified<Order>, exchangeCredentials: ExchangeCredentials): Promise<OrderInfo>;
skubarenko commented 6 years ago

I also propose to change the returned type of the deleteOrder method. I think the boolean type isn't consistent with other methods: if an order isn't deleted then it's better to throw Error as in other methods. Maybe, will it return an instance of the Order type? Order -> createOrder() -> Identified<Order> -> deleteOrder() -> Order Then, the deleteOrder will have the following signature:

deleteOrder(identifiedOrder: Identified<Order>, exchangeCredentials: ExchangeCredentials): Promise<Order>;
maxima-net commented 6 years ago

This method should get an instance of the Identified<Order> instead of an order id.

I completely agree with you.

deleteOrder(identifiedOrder: Identified<Order>, exchangeCredentials: ExchangeCredentials): Promise<Order>;

I think, it's better to return the void type. Because it isn't obviously that the deleteOrder method should return something.