Bug: improperly cloned expandable object

[ETFVoyageur]

By the way -- aside from my whims, this needs to be fixed. I am highly suspicious that improper cloning may be at the root of the bug that SimpleFont changes made in the Indicators dialog are not cancelable. (bug already reported)

Let's focus on getting this fixed and then we can see about the Cancel issue.

--EV

[bltdavid]

ICloneable is a failed experiment.

Cloning may still be an issue, so I wouldn't give up on that idea just yet.

Does SimpleFont.Clone() perform a deep copy or shallow copy?

That question should probably be directed to NT engineers.

In the meantime, you could try implementing your own deep copy cloning function.

Luckily, SimpleFont doesn't seem to have that many properties to deal with.

My apologies if these suggestions appear like grasping at straws.
(Lol, they probably are )

[bltdavid]

It's possible that SimpleFont is simply not designed to be expanded in a class that is itself already being expanded.

That is, the SimpleFont "nesting" that you're doing may be,

1. not implemented.
2. not implemented well (aka buggy).
3. not supported.

By any chance, can you point to product examples in the wild where nested expandable properties in a property grid behave well together?

I only ask because if an example software product exists that demonstrates correct behavior, then it easily follows that it must be possible to implement it correctly.

(Also, I can't think of any examples myself ... brain is fried)

[bltdavid]

Our default clone method is setup and works as you have found ...

Brett, I've read this several times now, and I'm still not sure ....

Are you referring to the indicator's internal Setup() method?

[ETFVoyageur]

I'm not tracking on the SimpleFont comments -- the issue is that NT fails to correctly clone MY class (BarNumbering). It just does the simplistic thing -- BarNumbering constructor, and then set the public properties. It does not call Clone(), even though I have now implemented ICloneable. Nothing about that has anything to do with SimpleFont. In the demo indicator the thing that does not get properly carried forward into the bogus cloned object is Parent -- that is a null reference in the cloned object because NT knows nothing about it (nor should it). That is why Clone() must be called.

The issue of nesting an expandable object keeps getting brought up, so I'll comment. It is a red herring in that it has nothing to do with why BarNumbering.Clone() is not called. Functionally there should be no problem -- cloning is just a recursive descent. Any object containing a SimpleFont just calls its Clone() method when it needs to. There is no real limitation on what a cloneable object can contain -- it just has to clone its children appropriately -- calling Clone() where available, otherwise assigning. Any non-constant child where assignment would be by reference had better implement Clone(). SimpleFont, for example, does.

FWIW: BarNumbering.Clone() uses SimpleFont.Clone() to propagate it. Whether that is deep or shallow is up to SimpleFont and is not related to the current issue (failure to clone BarNumbering).

FWIW: I just replaced the demo program I attached a few messages back. Clone() was cloning Parent, but not updating it appropriately. Did not really matter, since Parent is not used in the demo indicator, and I doubt most people would have noticed. Nevertheless I thought it should be accurate so I updated the file.

--EV

[ETFVoyageur]

Brett,

Are we on the same page now?

The issue is that there must be a way for NT to call Clone() on an expandable object that is the value of an expandable property. The logic is exactly the same as why indicators support Clone(). As far as I know there is no equivalent capability for expandable objects -- and if that is correct then that lack is a serious NT bug.

The indicator can clone it fine -- that is not the issue. The issue is when NT tries to clone it independently from any indicator. NT must call Clone() when that happens; currently it does not and I know of no way to get NT to do so.

--EV

[NinjaTrader_Brett]

I guess I am still not making myself clear.

"in the Indicator clone" -- stop right there. NT is calling the expandable object constructor directly. The indicator is not in the picture. The indicator is not being cloned at that time. Cloning my indicator works just fine today. The problem comes when NT tries to clone the expandable object directly, totally out of context of any indicator.

--EV

What is the scenario for this. I'm not following this. What is the scenario where NT would try to clone the expandable object directly? Since I can't think of a scenario where NT would try to do that outside the context of an Indicator/Strategy/MarketAnalyzer Column/ Etc. Everything comes from a NinjaScript object. Perhaps I'm missing something but this is the next key part to get under our belt as we work this problem out.

[ETFVoyageur]

Brett,

How to reproduce this is described in detail in the comments at the top of the demo indicator file I have attached. What happens is:

• Create a chart template that includes the demo indicator. The bug does not occur while creating the chart to store as the template.
• Create a new chart, using that template
• NT will construct a new, not cloned, indicator instance. In doing so the property constructor will be called from the indicator (good), and then the indicator constructor runs (good)

Now comes the bad part

• NT then directly calls the expandable object constructor. The indicator is not on the call stack. NT then sets the public properties of the object. This sequence is the bug -- NT attempting to create an instance out of whole cloth rather than using any legitimate cloning method, such as calling Clone(). Parts of the object other than the public properties do not get set up, such as a reference that remains null.
• NT then calls the indicator's set() method for this property, passing it the newly-created bogus object.

Reproducing this is quite easy by following the directions at the top of the demo file. It is 100% reproducible.

I have attached a new copy of the demo indicator, just to make finding it easy and guarantee that we are working from identical source.

--EV

[NinjaTrader_Brett]

Alright now we are getting there. Appreciate your persistence.

This is something completely different then what we have been discussing (and perhaps is my fault for heading us down that path so sorry). This is Restore() logic as we restore an instance from a template or workspace and works on the principle of XML serialization/deserialization. Since as we save an template or workspace we save it to an XML document and deserialize the document and restore it.

Let me get my head wrapped around that and will get back.

[ETFVoyageur]

Given what you are saying, perhaps you should include in your thinking the fact that the expandable object's type converter never gets called. I'm not sure -- just a thought.

It seems to me that there is a fundamental issue here -- that dynamic data cannot be serialized. Restoration needs to be done in some way that allows for this.

One thought -- perhaps the right method is to start by cloning a known good object, and then update it from the deserialization information?

Another way to do that, which sounds simpler and more foolproof to me, would be to just directly update the indicator's object rather than trying to make a new one to replace the indicator's object. That would keep everything in the context of the indicator, which is probably for the best.

That is not well thought out, so take it for what it is worth.

--EV

[bltdavid]

It seems to me that there is a fundamental issue here -- that dynamic data cannot be serialized.

That doesn't seem right. Of course it can be serialized.

Isn't serialization for complex/dynamic properties the programmer responsibility?
Did NT8 change that?

I mean, in NT7, you had to add a 2nd property to handle serialization for a "complex" type as simple as Color. And then the same had to be done with Font, Pen, etc, as well as any custom expandable properties, too.

Open up your .xml template file created by NT8 -- do you see all your indicator properties in there?

Is NT8 correctly serializing your BarNumbering and SimpleFont properties to that .xml file?

[ETFVoyageur]

Well, technically you can serialize anything, but that does not mean it will be valid when you restore it. Consider my demo program. What good would it do to serialize a reference to the property's parent indicator? That kind of thing needs to be reconstructed on the fly when you deserialize.

That's why deserializtion needs to be done in context, such as my suggestion the way to do it may be to just update the indicator's property rather than construct a new one to replace the indicator's property.

[bltdavid]

Well, technically you can serialize anything, but that does not mean it will be valid when you restore it. Consider my demo program. What good would it do to serialize a reference to the property's parent indicator? That kind of thing needs to be reconstructed on the fly when you deserialize.

All of which is the responsibility of the programmer, including any fix-up needed during deserialization.

[ETFVoyageur]

As Brett pointed out, NT does things in the context of the indicator. If they would also deserialize in that context, as I suggested, then the problem would disappear. Why not just do that -- very consistent with NT philosophy?

As is, the way they are doing it, at just which point would you suggest that the indicator has the opportunity to fix things (and how would it know it is supposed to do that)?

[NinjaTrader_Brett]

Ok, worked through it.

This is c# standard XML serialization and deserialization of an object. There is nothing special we are doing there. The rules of how XML serialization work is defined by Microsoft and can be found by googling c# / .net XML serialization/deserialization. This is completely separate then clone() so no need to include that in the thought process.

So what does this mean in your case:

* .Net only serialize 'basic' types by default. You would use the XMLIgnore attribute to disable serialization of ones you don't want serialized.
* You can look at IXmlSerializable and the various advanced XML Serialization write and restore options but thats outside the scope of what we support.
* The recommended/simple approach is that you disable serialization of that complex property, instead serialize the simple individual 'backing' properties. Then you could glue that back together as needed where you need it e.g. state.configure.

Ultimately lots of ways to skin the cat but its more c#/.net xml restore limitations your working with then anything we control in the NT framework.