What is the preferred way to add futures/swap support

[szmcdull]

trafficstars

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.

[vslee]

Are you talking about OKEx?

[szmcdull]

OKEx, Huobi, Binance, Deribit, KuCoin etc.

[vslee]

For OKEx, can you determine whether it's futures / swap from the symbol name? I haven't looked into he others.

[szmcdull]

Maybe futures/swap can be determined by symbol name. But margin/spot share the same symbol names and depths.

[vslee]

In that case, I think option (b) would be better and less confusing.

[jjxtra]

Which methods would need the parameter?

[vslee]

Actually, a better option would be to create a property in ExchangeAPI:

		protected bool isMarginTradingEnabled = false;

		public virtual bool IsMarginTradingEnabled
		{
			get { return isMarginTradingEnabled; }
			set { throw new NotImplementedException(
				"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.

[jjxtra]

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]

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.

[vslee]

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]

What would a derived class look like trying to set the bool to true? Would it need it's own bool?

[vslee]

The derived class would have this:

		public override bool IsMarginTradingEnabled
		{
			get { return isMarginTradingEnabled; }
			set { isMarginTradingEnabled = value; }
		}

[jjxtra]

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>
public virtual bool IsMarginTradingPossible => false;

private bool isMarginTradingEnabled = 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>
public bool IsMarginTradingEnabled
{
    get { return isMarginTradingEnabled; }
    set
    {
        if (!IsMarginTradingPossible)
        {
            throw new NotImplementedException(
	    "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;
    }
}

[vslee]

Yes, I like that

[jjxtra]

Awesome, sounds like a plan for whoever tackles this beast :)

[szmcdull]

How is isMarginEnabled related to futures/swap?

[vslee]

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.