DNSObject::ComputeFqdn rewrite

[Hipska]

Hi, the logic in the ComputeFqdn could be simplified. https://github.com/TeemIp/teemip-core-ip-mgmt/blob/205f164d28c3e208c7bcd0668f2e49da64a80e02/teemip-network-mgmt/datamodel.teemip-network-mgmt.xml#L57-L87

My suggestion would be either

$l = strlen($sZoneName);

$sName .= substr($sName, - 1) != '.' ? '.' : null;
$sName .= substr_compare($sName, $sZoneName, - $l, $l) != 0 ? $sZoneName : null;

or

$l = strlen($sZoneName);
if (substr($sName, - 1) != '.')
{
    $sName .= '.';
}
if (substr_compare($sName, $sZoneName, - $l, $l) != 0)
{
    $sName .= $sZoneName;
}

I don't know which would have your preference.

[Hipska]

The same count as well for _Domain::ComputeValues https://github.com/TeemIp/teemip-core-ip-mgmt/blob/205f164d28c3e208c7bcd0668f2e49da64a80e02/teemip-network-mgmt/_domain.class.inc.php#L220-L260 Can be rewritten as

$this->Set('name', static::ComputeFqdn($this->Get('name'), $this->Get('parent_name')));

parent::ComputeValues();

[Hipska]

@xtophe38 I can do the changes and make a PR, but would like to know your opinion on it before.

[xtophe38]

@Hipska, if we are simplifying, may as well use the shortest (though still meaningful) option, that is the 1st one for ComputeFqdn (I like the ternary operator anyway). And your second proposal makes sense as well.

So, go ahead with the PR. Thanks !

[Hipska]

May I ask why the ComputeFqdn method is defined in the datamodel xml and not in the _DNSObject class definition?

It would have prevented the need for dd8fa83561543956dcca82647605153cabcc1bf9 😅

[xtophe38]

PR "rebased and merged".

Well, not too sure anymore... :-) probably to allow the method to be overloaded by a child class through xml.

[Hipska]

Which would still be possible..

(PS: I don't really like the rebasing. It makes cleanup on my side much harder.)