Entity: some questions regarding [set|get]Parent()

[agarny]

1. I am struggling to see why we need to expose the Entity::setParent(). Are there use cases where a libCellML user would actually want to set the parent of an entity?
2. I find Entity::getParent() rather cumbersome to use. I have a use case where I have an encapsulated component for which I want to retrieve the (parent) model. With Entity::getParent(), I can retrieve my encapsulated component's parent component. But then, I have to retrieve that parent component's parent component until I eventually reach the parent model. This is rather tedious. I wish there was an Entity::getParentComponent() method that would return the parent component (should there be one) and an Entity::getParentModel() method that would return the parent model.
3. I find very risky to have Entity::clearParent() as part of the API.
4. Entity::hasParent() probably ought to be renamed Entity::hasParentComponent()?

[agarny]

A quick implementation of Entity::getParentComponent() and Entity::getParentModel() can be found here.

[hsorby]

If we know what we are getting why not return a smart pointer to the object instead?

If I remember correctly the getParent is used to check for circular parentage. If the tests still pass then I don't mind changing it (never did like what I did there anyway).

I think you have to have Entity::clearParent() don't you?

Entity::hasParent isn't the same as Entity::hasParentComponent the question being asked is not the same.

[agarny]

If we know what we are getting why not return a smart pointer to the object instead?

Agreed, it was just a quick and dirty implementation (at this stage) for issue #257.

If I remember correctly the getParent is used to check for circular parentage. If the tests still pass then I don't mind changing it (never did like what I did there anyway).

I have just implemented Entity::getParentComponent() and Entity::getParentModel() using a smart pointer (and had to modify many of our tests!), and it's all working fine except for the Connection.componentlessVariableInvalidConnectionClearParentCheck test. Rather than getting:

<?xml version="1.0" encoding="UTF-8"?>
<model xmlns="http://www.cellml.org/cellml/2.0#">
   <component name="component2">
      <variable name="variable2" />
   </component>
   <connection component_1="component2">
      <map_variables variable_1="variable2" variable_2="variable1" />
   </connection>
</model>

I am getting:

<?xml version="1.0" encoding="UTF-8"?>
<model xmlns="http://www.cellml.org/cellml/2.0#">
   <component name="component2">
      <variable name="variable2" />
   </component>
   <connection component_1="component2" component_2="component1">
      <map_variables variable_1="variable2" variable_2="variable1" />
   </connection>
</model>

It is clearly related to using a smart pointer rather than a raw pointer, but I can't pinpoint where the exact cause. Still, I would say that the new actual value should become the new expected value...?

I think you have to have Entity::clearParent() don't you?

Yes, if we want to use things like ComponentEntity::takeComponent(). Actually, the second version of ComponentEntity::takeComponent() doesn't clear the component's parent. Is that "normal"?

Entity::hasParent isn't the same as Entity::hasParentComponent the question being asked is not the same.

??

Entity::hasParent() takes a component as a parameter. So, I am merely suggesting making Entity::hasParent() consistent with Entity::getParentComponent() hence I think it should be renamed to Entity::hasParentComponent().