[core] Create channelUID instead of searching for the channel

[cweitkamp]

• Create channelUID instead of searching for the channel

Signed-off-by: Christoph Weitkamp github@christophweitkamp.de

[kaikreuzer]

@Hilbrand & @maggu2810 As you both did heavy refactoring of all this recently, any comment about this change? Looks pretty straight forward to me...

[J-N-K]

Mhm. The current implementation checks if the channel exists before checking if it is linked and returns immediately if it is not existing. So the question may be: is this check more or less efficient than searching for the link (i.e. how often is isLinked called for non existing channels and therefore this check saves resources for checking the link registry)

[cweitkamp]

For the current implementation of the AbstractLinkRgistry it does not matter if the channel really exists on a thing or not because it internally uses a HashMap to keep track of a reference between an UID and any existing links for it to an item. The check is very fast - a lot faster than querying the thing for the channel first.

[Hilbrand]

I noticed the construction with creating the object is used on more places in the class. I wouldn't prefer creating objects when not strictly needed. This method is probably used very often, so it would result in lots of object creation. But that might not be a big issue.

Furthermore, Is the creation in this change always correct? A channel can have a group id. Creating it like this would be without group Id unless the groupId is part of the string parameter channelId passed. Or is that always the case? Changing it here would assume that groupId is always part of it, which currently is not required as it just searched based on the id. For example a lot of calls in ESH alone look something like this: isLinked(channel.getUID().getId()). This doesn't include the group id. (I also wonder why not call it in the UID directly, which seems more efficient, legacy?)

[cweitkamp]

Thanks for your opinion on this. Deriving from it we have a couple of methods in the BaseThingHandler class which may lead to bugs if a binding uses channel groups. Nearly every important method has a version using the channel id as argument and create the channelUID from it. What do you propose? From my personal view we maybe should now leave this method like it is and deprecate it and the other ones.

legacy?

Yes, the isLinked(ChannelUID) version has been introduced one year ago.

[cweitkamp]

For example a lot of calls in ESH alone look something like this: isLinked(channel.getUID().getId()). This doesn't include the group id.

To clarify: During my investigations for #6736 (PR #6773) I realized that channel.getUID().getId() returns something like groupId#channelId. It includes the group id. Thus it should be no problem to use it like that.