You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
What is the preferred way to add futures/swap support #536
a) Add another virtual exchange that works only for futures, yet another one for swap.
b) Add a marketType parameter to all methods of the existing exchange.
The first one is closer to the native APIs provided by most exchanges.
The second one is more user friendly in my opinion.
Actually, a better option would be to create a property in ExchangeAPI:
protectedboolisMarginTradingEnabled=false;publicvirtualboolIsMarginTradingEnabled{get{returnisMarginTradingEnabled;}set{thrownewNotImplementedException("Margin trading is not implemented for this exchange. Please submit a PR if you would like to implement it.");}}
Any exchanges that do implement it can override the property to set the field isMarginTradingEnabled. This way, no new classes need to be created, and no method signatures need to be changed.
Would returning false be better, that way people don't have unexpected exceptions? I suppose if we document the setter with a NotImplementedException that may be sufficient?
isMarginTradingEnabled is by default set to false (I updated the code above to make it clearer). I think it is better to throw an execption, than to have ppl set it to true and thinking that they are using margin trading when they are not. This would be set in the initialization part of ppl's code anyways, so it's not like it would interrupt their trading in the middle of the day.
Also, the exception would only be thrown if someone tries to use margin trading where not implemented. All existing user code would still work fine w/o any changes.
I would propose the following, where derived classes opt in and override IsMarginTradingPossible to true if desired.
///<summary>Indicates if margin trading is possible. Derived classes can override and return true. The default is false.</summary>publicvirtualboolIsMarginTradingPossible=> false;privateboolisMarginTradingEnabled=false;///<summary>Gets whether margin trading has been turned on. If IsMarginTradingPossible is false, any attempt to set this value will throw a NotImplementedException.</summary>publicboolIsMarginTradingEnabled{get{returnisMarginTradingEnabled;}set{if(!IsMarginTradingPossible){thrownewNotImplementedException("Margin trading is not implemented for this exchange. Please submit a PR if you would like to implement it, derived classes can override IsMarginTradingPossible to return true .");}isMarginTradingEnabled=value;}}
isMarginEnabled is just for whether the user wants to use margin vs spot. Since the symbol names for futures/swap are different, those can be added to the existing methods without adding any parameters.
Activity
vslee commentedon Apr 16, 2020
Are you talking about OKEx?
szmcdull commentedon Apr 16, 2020
OKEx, Huobi, Binance, Deribit, KuCoin etc.
vslee commentedon Apr 16, 2020
For OKEx, can you determine whether it's futures / swap from the symbol name? I haven't looked into he others.
szmcdull commentedon Apr 20, 2020
Maybe futures/swap can be determined by symbol name. But margin/spot share the same symbol names and depths.
vslee commentedon Apr 20, 2020
In that case, I think option (b) would be better and less confusing.
jjxtra commentedon Apr 20, 2020
Which methods would need the parameter?
vslee commentedon Apr 22, 2020
Actually, a better option would be to create a property in
ExchangeAPI:Any exchanges that do implement it can override the property to set the field
isMarginTradingEnabled. This way, no new classes need to be created, and no method signatures need to be changed.jjxtra commentedon Apr 22, 2020
Would returning false be better, that way people don't have unexpected exceptions? I suppose if we document the setter with a NotImplementedException that may be sufficient?
vslee commentedon Apr 22, 2020
isMarginTradingEnabledis by default set to false (I updated the code above to make it clearer). I think it is better to throw an execption, than to have ppl set it to true and thinking that they are using margin trading when they are not. This would be set in the initialization part of ppl's code anyways, so it's not like it would interrupt their trading in the middle of the day.vslee commentedon Apr 22, 2020
Also, the exception would only be thrown if someone tries to use margin trading where not implemented. All existing user code would still work fine w/o any changes.
jjxtra commentedon Apr 22, 2020
What would a derived class look like trying to set the bool to true? Would it need it's own bool?
vslee commentedon Apr 22, 2020
The derived class would have this:
jjxtra commentedon Apr 22, 2020
I would propose the following, where derived classes opt in and override IsMarginTradingPossible to true if desired.
vslee commentedon Apr 22, 2020
Yes, I like that
jjxtra commentedon Apr 22, 2020
Awesome, sounds like a plan for whoever tackles this beast :)
szmcdull commentedon Apr 24, 2020
How is isMarginEnabled related to futures/swap?
vslee commentedon Apr 24, 2020
isMarginEnabled is just for whether the user wants to use margin vs spot. Since the symbol names for futures/swap are different, those can be added to the existing methods without adding any parameters.