Collection was modified related question

[soulfx]

Hello, i started a separate thread here as in the past i sensed annoyance from others at hijacking an existing thread 'collection was modified' ; so i elected to start a separate one.

In any event. I appreciate tradersmarts sharing of his testing and his testing of 5 scenarios of iterating through dictionaries. I have a similar situation where i collect data in onmarketdate() and add to dictionaries and in Onrender am output the data in the dictionary,

I also over time get intermittent error message very similar to tradersmarts ;
collection was modifed ......
but also my OnRender is only outputting the data and not modifying the content in the foreach loop, tradersmarts confirmed this was the case but Ninja support still advised using for loop or the foreach with a tolist() operation on the dictionary.

Now i note in the standard ninja provided samples namely VolumeProfile which i did reference in the past as a guide to perhaps 'best practices' and this uses foreach over dictionaries. So my question is why this is illustrated in a ninja provided sample when it is perhaps not the best practice as the onmarketdata() in that code could be updating information that onRender is iterating over?

I just wanted to be clear on this and i will review and perhaps modify to for loops using for my onrender processing.

Does the toList() option actually copy the dictionary so whilst processing it it has no effect on the original dictionary? are there memory considerations to using this when there are many dictionaries to iterate for each of the bars in the window in onrender?

Again this error appears now and again and like tradersmarts is difficult to reproduce.

Would the lock statement he tried .. make a difference if used in the onmarketdata or where the dictionary is being modified vs just looped through as in onrender?

Again the lock is not used in your sample VolumeProfile; so was just wondering on that.

I occassionally am getting Onrender issues and from tradersmarts post it has given me an insight perhaps to the foreach potential issue and whether this triggers other errors too.

over time through beta the subtleties of the errors and the vague incomplete error logging make it challenging to narrow down the issue - especially when the errors are intermittent.

thanks

[NinjaTrader_PatrickH]

Hello soulfx,

Thank you for your post.

ToList() would use a cached copy of the dictionary. There should not be anything specific to consider as far as performance here. However, a large number of loops/queries in OnRender may not be ideal in certain cases. Generally speaking, OnRender should only be used for rendering and not for calculating values. It would be best to run the function you need (such as OnMarketData) when making calculations.

Please let me know if you have any questions.

[soulfx]

Hello, yes most calculations are done outside of onrender other than those for positioning of drawing objects which can mostly only be done in onrender.

What is not clear (before i make alot of changes in a new version of my custom code) - is if i need to do this IF my foreach loops are not modifying the data in the dictionary ie is read only. OR is this being recommended as per the thread tradersmarts discussed with Jesse? Again the sample VolumeProfile does not do this ?

Is this a case of try it it might help or is recommended ? It seems a bit vague other than it helped tradersmarts intermittent error but like him nothing is being modified.

How about the lock :
lock (e.Instrument.SyncMarketData)
is this recommended in this instance i described below ?
what situation would this be needed?


in the other thread collection modified post 6 Jesse wrote :

A foreach loop would not be safe to use when modifying a collection that it is iterating over, a for loop on the other hand has a specific index supplied with each iteration and can change in the case of a collection change. using a seperated collection that is not directly a pointer to the actual collection can work as well which would be the .ToList() statement.

did he/she (sorry not sure of name) mean if modified within the foreach or meant if perhaps modified outside of the loop ie in onmarketdata whilst the dictionary was being read. This is the part where wasnt clear. any clarification on this statement?

thanks

[tradesmart]

My dictionaries were not being modified in OnRender(); merely iterated over via foreach. They are created and updated continuously in OnMarketDepth(). Evidently an update took place in OnMarketDepth() during the reading of that same dictionary in OnRender(). At least, that's my guess as to the cause of the intermittent "Collection was modified" error.

[soulfx]

Thanks for reply and yes i understood that to be the case and is the same for me my for each loops in OnRender are not modifying anything from the collection - apologies if i was implying your loops in onrender were modifying - our context is very similar which is why i related it to your original thread.
Will await clarification from ninja but also try the other for loop suggestions ; the point i had was the volumeprofile example was what i followed in some of my design but now i/we hear this may not be sound given the multi threaded environment and with collections.
thanks

[tradesmart]

I, too, used various built-in indicators as exemplars for mine. However, when there's a "best practices" discrepancy between code in an @indicator.cs file (e.g., @VolumeProfile.cs and @SampleCustomRender.cs) and that provided in the help guide (e.g., http://ninjatrader.com/support/helpG.../rendering.htm), I've come to the conclusion that the help guide is probably the most up-to-date and the one to emulate.

It becomes a bit of a moving target, since the recommendations in the help guide change without notice and more frequently than new releases drop.

NT staff: would it be possible somehow to alert us to changes in best practices? I've been using WinMerge to compare prior release code with new release code to see if any changes might affect my indicators, but I now see that this is not enough.

[soulfx]

Hello i appreciate the next reponse is in the queue along with other threads, and thus i await the reponse in due course. I just wanted to be sure in addition to tradersmarts question that my questions lower down and the use of lock .... and the synch method illustrated in the sample market depth example - and when and why this is not in the help guide ie it currently remains undocumented.
The reason i say this as there can be a tendency at times to overlook other questions being asked and these are important questions and considerations given the 'confusion' that has been generated with respect to onbarupdate/onmarketdata order of being called with ticks and that some of the documention is being updated.
thanks

[NinjaTrader_PatrickH]

Hello soulfx,

Thank you for your patience.

What my colleague Jesse referenced in your quoted reply is the same as what is seen in the Volume Profile.

A foreach loop would not be safe to use when modifying a collection that it is iterating over, a for loop on the other hand has a specific index supplied with each iteration and can change in the case of a collection change. using a seperated collection that is not directly a pointer to the actual collection can work as well which would be the .ToList() statement.

At line 121 in the Voluem Profile you will see the following:

sortedDicList.Add(cacheDictionary);

sortedDicList is a list of the cacheDictionary. So we are calling the List that is not a direct pointer of the original dictionary.

Lock would not be the recommended way to do this, please refer to line 121 as listed above in the Volume Profit indicator's code.

Please let me know if you have any questions.

[NinjaTrader_PatrickH]

I, too, used various built-in indicators as exemplars for mine. However, when there's a "best practices" discrepancy between code in an @indicator.cs file (e.g., @VolumeProfile.cs and @SampleCustomRender.cs) and that provided in the help guide (e.g., http://ninjatrader.com/support/helpG.../rendering.htm), I've come to the conclusion that the help guide is probably the most up-to-date and the one to emulate.

It becomes a bit of a moving target, since the recommendations in the help guide change without notice and more frequently than new releases drop.

NT staff: would it be possible somehow to alert us to changes in best practices? I've been using WinMerge to compare prior release code with new release code to see if any changes might affect my indicators, but I now see that this is not enough.

Hello tradesmart,

Thank you for your post.

I will forward this as a suggestion to our Product Management team as there is no current means to alert you on changes to the Help Guide.

[soulfx]

collection modified error again with plot series

Hello this happened again today but on a different indicator which was named ....

In the State.SetDefaults i have ...

AddPlot(new Stroke(Brushes.Transparent, 1), PlotStyle.Line, "xxC");
AddPlot(new Stroke(Brushes.Transparent,1), PlotStyle.Line, "xxH");
AddPlot(new Stroke(Brushes.Transparent,1), PlotStyle.Line, "xxL");
AddPlot(new Stroke(Brushes.Transparent,1), PlotStyle.Line, "xxO");

within OnMarketData and OnBarUpdate .. these values are refernced and updated
mostly in OnBarUpdate

eg xxH[0] = value;
xxC[0] = value2; etc

then in OnRender .. in the standard way to iterate over visible chart bars ...

the values are accessed

xxCValue = xxC.GetValueAt(idx);

in the normal way....

this indicator i have had running for many months and occassionally get this error but did again today ... error on calling 'onstatechange' collection was modified ...

these values are not modified in onrender nor in any of the state sections in OnStateChange

so given these are not dictionaries of lists .......

how do i handle this .. as this is just Plotseries values that are then read/accessed in the onrender to draw some other information

any input and guidelines or best practice appreciated ......

i havent had the error recently tradersmart has had on my dictionary indicator but am still looking at modifying potentially

thanks

[NinjaTrader_PatrickH]

Thank you for your update, soulfx.

Do you have an indicator we could test that produces the same behavior?

I look forward to your response.

[NinjaTrader_PatrickH]

NT staff: would it be possible somehow to alert us to changes in best practices? I've been using WinMerge to compare prior release code with new release code to see if any changes might affect my indicators, but I now see that this is not enough.

This request has been assigned the internal tracking id SFT-1931.