Make some members of TVirtualNode private

[joachimmarder]

Frequently users of Virtual TreeView have problems because they use the members of TVirtualNode directly instead of the proper methods in TBaseVirtualTree. See e.g. #831

We should consider making them private, we could replace them with nice getter functions or properties, which are implmeneted similar to those in the TBaseVirtualTree. This has already been done for the Data field.

[pyscripter]

+1

[obones]

Isn't this a duplicate of #823 ?

[joachimmarder]

Yes, its a duplicate. Forgot about that.

[livius2]

If realy considered i will change it not to be private only as protected.
You can not imagine what users can do with VT, limiting this to private will be not beneficial.

[joachimmarder]

TVirtualNode is a record, so protected won't make any difference.

You can not imagine what users can do with VT

I can! I get the emails if that stuff doesn't work as expected. Or if they simply did not recognize that TVirtualNode.Parent is an internal piece of data that should better not be touched without using the appropriate setter.

[Fr0sT-Brutal]

Or if they simply did not recognize that TVirtualNode.Parent is an internal piece of data that should better not be touched without using the appropriate setter.

I found myself using TVirtualNode.Parent pretty frequently (as a read-only property though). I also use Index and States which currently are irreplaceable by a tree method. And tree.NodeParent[Node] is inconvenient in some cases when you only have a node variable provided that there's no TVirtualNode.Owner property.
So I vote for hiding important members but exposing getters for them and maybe replace tree.NodeParent[node] with node.GetParent or at least node.GetOwner.NodeParent[node]

[joachimmarder]

Now, with the units split up, we can't use friend access of the control classes on private members of TVirtualNode anymore. That means we will also need public setters, we just could not use them in the properties and leave them read-only. Any further thoughts on this from the watchers of this issue? Is there still enough benefit to justify this change?

[obones]

Well, to me, as a general rule, every field should be private and only accessed via properties.
I have not checked if all such properties would need a "write" accessor though but if that's not the case, I would only add write wherever needed, leaving the other read only.

[joachimmarder]

I now did this for TVirtualNode.Parent.

[joachimmarder]

I will stop my work here. The members that typically cause support are now private with public readonly properties and a public setter function, which is now necessary after all the units have been split up.