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 cleanup+example (WIP) #80

Closed BunkerCoder closed 4 years ago

BunkerCoder commented 4 years ago

If this looks correct, I'll start on testing (new example looking good). TIA for the review.

mathpaquette commented 4 years ago

@BunkerCoder look also 1 test is failing. https://ci.appveyor.com/project/mathpaquette/iqfeed-csharpapiclient/builds/33518962#L873

mathpaquette commented 4 years ago

Something like this is good enough isn’t? https://github.com/mathpaquette/IQFeed.CSharpApiClient/commit/ce9d2c300429fef07d0049b0340f21980a922654

From: BunkerCoder notifications@github.com Sent: Monday, June 15, 2020 10:33 AM To: mathpaquette/IQFeed.CSharpApiClient IQFeed.CSharpApiClient@noreply.github.com Cc: Mathieu Paquette me@mathpaquette.com; Comment comment@noreply.github.com Subject: Re: [mathpaquette/IQFeed.CSharpApiClient] Dynamic cleanup+example (WIP) (#80)

@BunkerCoder commented on this pull request.


In src/IQFeed.CSharpApiClient/Streaming/Level1/Level1DynamicFields.cshttps://github.com/mathpaquette/IQFeed.CSharpApiClient/pull/80#discussion_r440219619:

@@ -223,7 +225,7 @@ public class Level1DynamicFields

     public static Level1DynamicFields Parse(string message, DynamicFieldset[] fields)

     {

message contains the raw feed data. From docs "The first field is the Message ID which will be a capital 'Q' for an Update message, or a capital 'P' for a Summary message. The second field is the symbol being updated." Did you notice the comments about Symbol required to be first? It's easy enough to code around and make it tolerant. How do you want to handle?

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/mathpaquette/IQFeed.CSharpApiClient/pull/80#discussion_r440219619, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABBICN2G7BKOAMS2VHELKI3RWYWI7ANCNFSM4N5ZLUPA.

mathpaquette commented 4 years ago

Instead of doing this I guess you can do:

replace for (var i = 0; i < fields.Length; i++)

to: for (var i = 1; i < fields.Length; i++)

Just to avoid creating 2 arrays. SplitFeedMessage() == create an array Skip(1) == create another array.

Thanks.

From: BunkerCoder notifications@github.com Sent: Monday, June 15, 2020 10:33 AM To: mathpaquette/IQFeed.CSharpApiClient IQFeed.CSharpApiClient@noreply.github.com Cc: Mathieu Paquette me@mathpaquette.com; Comment comment@noreply.github.com Subject: Re: [mathpaquette/IQFeed.CSharpApiClient] Dynamic cleanup+example (WIP) (#80)

@BunkerCoder commented on this pull request.


In src/IQFeed.CSharpApiClient/Streaming/Level1/Level1DynamicFields.cshttps://github.com/mathpaquette/IQFeed.CSharpApiClient/pull/80#discussion_r440219619:

@@ -223,7 +225,7 @@ public class Level1DynamicFields

     public static Level1DynamicFields Parse(string message, DynamicFieldset[] fields)

     {

message contains the raw feed data. From docs "The first field is the Message ID which will be a capital 'Q' for an Update message, or a capital 'P' for a Summary message. The second field is the symbol being updated." Did you notice the comments about Symbol required to be first? It's easy enough to code around and make it tolerant. How do you want to handle?

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/mathpaquette/IQFeed.CSharpApiClient/pull/80#discussion_r440219619, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABBICN2G7BKOAMS2VHELKI3RWYWI7ANCNFSM4N5ZLUPA.

BunkerCoder commented 4 years ago

I think this addresses concerns and issues. If it looks OK, I'll complete the test TODOs.

BunkerCoder commented 4 years ago

Possibly, maybe done. Thanks again for your help.