Announcement
Collapse
No announcement yet.
Partner 728x90
Collapse
NinjaTrader
Execution sequence for my indicator -- need help
Collapse
X
-
Originally posted by ETFVoyageur View Post
-
Thanks for taking the time to write feedback on the design. I'll forward it to the development team.
To set expectations correctly, the architecture of NT8 is pretty well set in stone. We are in beta working to improve product stability to be able to make a release and we're not at a point in the development cycle where we could make architectural changes of this nature.I would suggest you start work to adjust any code that you normally would run in the constructor to work in State.SetDefaults appropriately since with NinjaTrader 8 this is how the indicator framework is setup and intended to be used.
Comment
-
Originally posted by koganam View PostWe must be talking at cross-purposes. That is what I have trying to say. Your constructor has not completed creating your object, but the base.SetState() can be still called, and is. That is what is on the stack, because your object has not yet overridden the base event handler/method, which it will first inherit, then override.
What do you think of my suggestion that they get rid of the SetDefaults call to OnStateChanged() and advocate setting defaults in the constructor? From the indicator's point of view getting to State.SetDefaults is not a state change -- it is the initial state when its constructor gets to run.
--EV
Comment
-
Originally posted by NinjaTrader_Brett View PostThanks for taking the time to write feedback on the design. I'll forward it to the development team.
To set expectations correctly, the architecture of NT8 is pretty well set in stone. We are in beta working to improve product stability to be able to make a release and we're not at a point in the development cycle where we could make architectural changes of this nature.I would suggest you start work to adjust any code that you normally would run in the constructor to work in State.SetDefaults appropriately since with NinjaTrader 8 this is how the indicator framework is setup and intended to be used.
- Ignore the premature call to OnStateChanged() for State.SetDefaults
- Set any needed defaults in its constructor -- as you have explained, the state will be fine for that.
Is there some reason why this would not be a good way to satisfy both of us for now -- the need to get the product out the door (with which I heartily agree) and the indicator's need for some minor initialization prior to setting defaults.
--EV
Comment
-
Originally posted by ETFVoyageur View PostI have no problem with the NT constructor calling anything (of its own) that it wants to, including base.SetState(). I have a huge problem with that having the side effect of calling into code in my raw instance (its constructor has not run).
What do you think of my suggestion that they get rid of the SetDefaults call to OnStateChanged() and advocate setting defaults in the constructor? From the indicator's point of view getting to State.SetDefaults is not a state change -- it is the initial state when its constructor gets to run.
--EV
On the other hand, if base.SetState() is being run merely to set the state to the required initial state, preparatory to your object constructor returning, so that the framework can get to work immediately after your constructor returns, then that sounds like a framework optimization. It would then appear that the framework is running SetState() to ensure things are kosher, and then putting the system into required initial state, even before the constructor returns. That way it can start SetDefaults()ing as soon as the constructor returns, because the call to OnStateChange() is now trivial and redundant, (and probably not even called again. I cannot be sure of that as I have not bothered to actually profile the process, so that is just speculation).Last edited by koganam; 08-04-2015, 02:40 PM.
Comment
-
Originally posted by koganam View PostSo have you actually coded anything in your State.SetDefaults() and confirmed that that code is running before the constructor returns? If it is actually running code before the constructor returns, yes, that would be very bad, design decision or not.
On the other hand, if base.SetState() is being run merely to set the state to the required initial state, preparatory to your object constructor returning, so that the framework can get to work immediately after your constructor returns, then that sounds like a framework optimization. It would then appear that the framework is running SetState() to ensure things are kosher, and putting the system into required initial state, even before the constructor returns.
That way it can start SetDefaults()ing as soon as the constructor returns
because the call to OnStateChange() is now trivial and redundant
(and probably not even called again. I cannot be sure of that as I have not bothered to actually profile the process, so that is just speculation).
There is a lot to like about NT8, but this specific thing is a botch. Taking Brett at his word that it won't be fixed this late in the Beta, I strongly suggest that the best way to have clean indicator code is to
- Ignore calls to OnStateChanged() when the state is State.SetDefaults
- Set your defaults from your constructor -- that will be after the state is State.SetDefaults, as Brett has confirmed and as I have confirmed with the debugger.
I really do hope they will fix the botch before release, though. I appreciate their time-to-market need, but waiting to fix it is also a problem because it could be a code-breaking change.
--EV
Comment
-
Originally posted by NinjaTrader_Brett View PostI don't see any blockers currently why that would not work for you to do. For that purpose you could basically ignore SetDefaults completely. Sounds like you could give that a try to head down the path you wanted to in the near term.
I trust you noticed that Koganam now agrees with me
If it is actually running code before the constructor returns, yes, that would be very bad, design decision or not. Indeed to run anything in an object that has not yet been created borders on malware behavior.
BTW: what's up with OnStateChanged() getting called twice before the constructor has returned, both times with the same state (SetDefaults)? Clearly the second time is not a state change. You should at least fix that, or you risk a problem with a developer who cannot tolerate the redundant (erroneous?) call.
--EV
Comment
-
Originally posted by NinjaTrader_Brett View PostOn the BTW comment. I'm not seeing that OnStateChanged getting called twice over here. Whats the scenario can you reproduce on your side with SMA indicator? If so whats different in your code that would help me narrow down to look into what is occurring?
--EV
Comment
-
Originally posted by koganam View PostSo have you actually coded anything in your State.SetDefaults() and confirmed that that code is running code in the object, before the constructor returns? If it is actually running code before the constructor returns, yes, that would be very bad, design decision or not. Indeed to run anything in an object that has not yet been created borders on malware behavior.
Constructor called but not completed with onstatechange running before constructor completion.
The other is/was the onstatechange called before the constructor to set defaults.
I will ask is this a multi threading issue if it is called before completion (of that is really happening).
Sorry if I'm really missing the boat here.
Comment
-
Originally posted by sledge View PostI'm reading multiple things in here.
Constructor called but not completed with onstatechange running before constructor completion.
The other is/was the onstatechange called before the constructor to set defaults.
I will ask is this a multi threading issue if it is called before completion (of that is really happening).
Sorry if I'm really missing the boat here.
- This is an NT design bug, not a multi-threading issue
- The constructor calls percolate up to the NT constructor. No indicator constructor gets to do anything but pass the call along. In particular no indicator constructor code gets to run yet.
- The NT constructor decides the State is now SetDefaults, and calls the indicator's OnStateChange(). Note that none of the indicator's constructor code has yet run, so NT is making a method call on an object that has not yet been constructed. In the opinion of at least some of us that is Pure Evil -- a major bug in NT
- The NT constructor code returns. At that point the indicator constructor code finally gets to run.
My thoughts about this:
- It is purely Evil. No matter what code you think has what responsibility, making a call on an object that has not yet been constructed is a major bug. NT is doing so deliberately, so that makes it a major design bug.
- While it remains a bug in any case, the bug may not affect you if you do not have a constructor that does something that needs to be done before OnStateChange() runs. In my case I do have such a thing. Most indicators do not.
- There are easier and harder ways for NT to fix the problem, but Brett has indicated that NT is unlikely to fix their bug before release, so anyone who cares needs to figure out how to work around it.
- I have been back and forth on how to deal with the NT bug. One thing to note when you get to thinking about it is that the State is guaranteed to be State.SetDefaults by the time the first indicator constructor code gets to run (at least for the current implementation).
Indicator solutions
- You could ignore calls to OnStateChange() for SetDefaults and do what you would have done there in your constructor. The State will be correct and you will be operating in a conventional manner. Brett has agreed that he sees no problem with doing this. My only concern is the future -- are you exposing yourself to problems once they do get around to fixing this bug?
- What I finally decided to do is a little more complicated, but I think more robust in the future if NT changes what it does. Here it is, and it seems to be working fine for me.
- OnStateChange() checks to see whether or not the constructor has completed. If construction has completed, it proceeds normally; if not, it just sets a flag and returns.
- When the constructor has finished its work and is about to return it checks that flag. If the flag is set the constructor calls OnStateChange() itself, replacing the ignored call.
- I feel that doing it this way is the way that is least likely to have future problems. It works today, and it will Just Work if NT fixes their bug.
FWIW: Mr. K and I have both observed that the call to OnStateChange() for SetDefaults is arguably a silly wasted call. The fact is that by the time any indicator constructor code gets to run the state is already SetDefaults, so the wasted call could just be omitted all together and developers told to do their SetDefaults work in the constructor.
--EVLast edited by ETFVoyageur; 08-04-2015, 06:50 PM.
Comment
-
Originally posted by ETFVoyageur View PostThanks for your reply. I'll give that a try. I do not see any downside.
I trust you noticed that Koganam now agrees with me
It really is a bad design that needs to get fixed. The fix I suggest is very simple, but would break code -- better now than after release. There are fixes that should not break code, such as waiting until after the constructor returns before calling OnStateChanged(), but they would probbly be more work for NT developers.
BTW: what's up with OnStateChanged() getting called twice before the constructor has returned, both times with the same state (SetDefaults)? Clearly the second time is not a state change. You should at least fix that, or you risk a problem with a developer who cannot tolerate the redundant (erroneous?) call.
--EV
Maybe I will write some code to specifically profile what is happening to see if I can get to the root of it, albeit given that even when I supply an instance constructor, I really do nothing more than populate initial values in it, this would hardly impact me.
Still, it is strange to be seeming to run code in an object that technically does not yet exist. For now, I will have to suspend judgement on what is actually happening, but I still stand by the statement that it is bad, if true.
Just my $0.02.Last edited by koganam; 08-05-2015, 05:24 PM.
Comment
Latest Posts
Collapse
Topics | Statistics | Last Post | ||
---|---|---|---|---|
Started by tony_28217, Today, 07:04 PM
|
0 responses
3 views
0 likes
|
Last Post
by tony_28217
Today, 07:04 PM
|
||
Started by flybuzz, Today, 10:33 AM
|
1 response
9 views
0 likes
|
Last Post
by flybuzz
Today, 06:59 PM
|
||
Started by spencerp92, 10-10-2023, 09:56 AM
|
4 responses
304 views
0 likes
|
Last Post
by flybuzz
Today, 06:45 PM
|
||
Started by samish18, Yesterday, 10:13 AM
|
1 response
25 views
0 likes
|
Last Post Today, 06:15 PM | ||
Started by Austiner87, Today, 05:02 PM
|
0 responses
7 views
0 likes
|
Last Post
by Austiner87
Today, 05:02 PM
|
Comment