mathpaquette / IQFeed.CSharpApiClient

IQFeed.CSharpApiClient is fastest and the most well-designed C# DTN IQFeed socket API connector available
MIT License
120 stars 43 forks source link

Dynamic Fieldsets v2 #73

Closed BunkerCoder closed 4 years ago

BunkerCoder commented 4 years ago

@mathpaquette - thanks again for the great advice. Here is v2. A few notes: Strong-typed code was too tedious/error-prone to write, so created a generator. Set it up as an example instead of a separate project because it needs a -define (for ApiClient and Examples) that toggles FieldsetDescriptionAttribute between private and public. Still haven't created tests, but existing tests pass. New StreamingLevel1DynamicExample example looks like it is pulling all feed fields correctly (still needs testing). Removed "Symbol" from dynamic fields; it doesn't need to be included as any requests, and is always included as first data field, so it's not really dynamic. Would have caused need for special-case code. "Type" field throws an error - probably because it's already in the data. Commented it out (with the error) from dynamic fields. Changed SelectUpdateFieldName to private; using it with the non-dynamic L1Client will break the client. I don't think it is useful to call it "mid-session". Just create a separate client. Didn't include the handler for S,CURRENT UPDATE FIELDNAMES that I did in the prior version; it's not needed, and could create a race condition. Appreciate your time.

BunkerCoder commented 4 years ago

@mathpaquette - Thanks for the thumb's up! Nervously awaiting your feedback... Take all the time you need.

mathpaquette commented 4 years ago

lol I was about to review it :)

mathpaquette commented 4 years ago

thats not bad but unfortunately not in a mergeable state Ill say. Let me explain:

  1. Level1MessageHandler doesnt have to be changed to handle that. We need to create an alternative strategy if there's a need to handle DynamicFields. So in other words. we need a new Level1MessageDynamicHandler
  2. I dont like the fact that we ending up with SummaryDynamic/UpdateDynamic looks confusing and clunky
  3. There's no way we need to pass DynamicFields in the Factory.

changes Im proposing:

  1. create a new Level1MessageDynamicHandler passing it in the Factory as a new param.
  2. rename UpdateSummaryMessageDynamic to Level1DynamicFields
  3. create a new property in UpdateSummaryMessage DynamicFields something like
    private Level1DynamicFields _level1DynamicFields;
    public Level1DynamicFields DynamicFields
    {
    get
    {
        if (_level1DynamicFields == null)
            throw new Exception("Must use Level1MessageDynamicHandler");
        return _level1DynamicFields;
    }
    }
  4. Add a new constructor in UpdateSummaryMessage to support this new property
mathpaquette commented 4 years ago

@BunkerCoder Ill help you creating the handler part. Join the chat is you need to talk more.

mathpaquette commented 4 years ago

@BunkerCoder https://github.com/mathpaquette/IQFeed.CSharpApiClient/commit/04459e013e09f87235d64d6390a9e1f5608876b2

mathpaquette commented 4 years ago

@BunkerCoder added more skeleton code now. you just need to feel the blanks!! https://github.com/mathpaquette/IQFeed.CSharpApiClient/commit/9e5fe278fa89ac71ab9c0de4f36c3dbc1ab15d45

BunkerCoder commented 4 years ago

@mathpaquette - great stuff. I'll dig in.

mathpaquette commented 4 years ago

@mathpaquette - great stuff. I'll dig in.

ill give you more examples soon.

mathpaquette commented 4 years ago

https://docs.microsoft.com/en-us/dotnet/standard/net-standard#net-implementation-support

mathpaquette commented 4 years ago

@BunkerCoder I managed the feature. Ill share code later today. I'll request your help to unit test this.

BunkerCoder commented 4 years ago

@mathpaquette - awesome! Looked like you were almost there, so figured you might finish it off. Happy to help test.

mathpaquette commented 4 years ago

@BunkerCoder cool, I really need your help on the testing part. Let me sharing with you later.