bchavez / Coinbase.Pro

:chart_with_upwards_trend: A .NET/C# implementation of the Coinbase Pro API.
https://docs.pro.coinbase.com/
Other
66 stars 27 forks source link

Why does subscribe mutate the subscription object #39

Closed granthoff1107 closed 3 years ago

granthoff1107 commented 3 years ago

So I decided to look into the subscription methods on the coinbase pro socket to be aware of whether I need to call them from the Dispose method.

I noticed something interesting, that we mutate the options when sending them into the socket.

  public async Task SubscribeAsync(Subscription subscription)
  {
     //...
     //Why do we mutate this instead of creating a Copy?  What happens if someone reuse the same subscription for multiple sockets?
     subscription.ExtraJson.Add("type", JToken.FromObject(MessageType.Subscribe));

     // ...
     this.RawSocket.Send(subJson);
  }

  public void Unsubscribe(Subscription subscription)
  {
     //Is this safe to call if the subscription does not have an unsubscribe token?
     subscription.ExtraJson.Add("type", JToken.FromObject(MessageType.Unsubscribe));

     var json = JsonConvert.SerializeObject(subscription);

     this.RawSocket.Send(json);
  }

Why do we mutate the existing options? What happens if there is no Unsubscribe JToken to be removed from the subscription?

bchavez commented 3 years ago

I don't remember the exact reason why, but it probably had to do with semantics from an API usage perspective. IE: If we want some notion of "subscribe" and "unsubscribe" in the API; then, what do you do when someone gives you the following object (when someone probably makes a mistake):

var subObject = new Subscription{ Type = MessageType.Unsubscribe }; // probably an error
socket.Subscribe( subObject ); //when really it should be a subscription; not an unsubscription.

So we forcibly set the "type" in association with the appropriate method call to ensure correct behavior.

We could probably subclass, and apply some type safety, but I didn't want to derive my own types too much away from the "official" API.

What happens if there is no Unsubscribe JToken to be removed from the subscription?

If type does not exist; one will be added. If the type exists, the current type is overwritten before being sent over the socket. The sub/unsub methods ensure that the correct type corresponding to the method call is always correct before being sent over the socket.