Added methods to find broadcastAddress(es), useful in bindings for discovery. #3747
Conversation
discovery. Signed-off-by: Mark Herwege <[email protected]>
|
As suggested by @kaikreuzer in review of openHAB Niko Home Control binding #1589. |
| */ | ||
| public static List<String> getAllBroadcastAddresses() { | ||
| try { | ||
| List<String> broadcastAddresses = new ArrayList<String>(); |
There was a problem hiding this comment.
Isn't a LinkedList better if we don't know the size (to reduce reallocations and copies)?
There was a problem hiding this comment.
Replaced with a LinkedList.
| @@ -55,9 +60,53 @@ public static String getLocalIpv4HostAddress() { | |||
| } | |||
| return hostAddress; | |||
| } catch (SocketException ex) { | |||
| LOGGER.error("Could not retrieve network interface: " + ex.getMessage(), ex); | |||
| LOGGER.error("Could not retrieve network interface: {}", ex.getMessage(), ex); | |||
| /** | ||
| * Get all broadcast addresses on the current host | ||
| * | ||
| * @return list of broadcast addresses |
There was a problem hiding this comment.
If the SocketException is thrown you return null.
So, this should be stated in the JavaDoc.
But wouldn't it make sense to throw the exception, so the caller is notified about the error -- or if the caller cannot handle it at all (does he need to know about the error) return an empty list?
There was a problem hiding this comment.
Returning an empty list, and documented.
| final List<String> broadcastAddresses = getAllBroadcastAddresses(); | ||
| for (String current : broadcastAddresses) { | ||
| if (broadcastAddress != null) { | ||
| LOGGER.warn("Found multiple broadcast addresses - ignoring {}", broadcastAddress); |
There was a problem hiding this comment.
The JavaDoc state "get the first candidate", so I don't see any reason to throw a warning if there are multiple candidates. The function should return the first one and not check if there is more then one, or should it?
There was a problem hiding this comment.
I used the same structure as in the getLocalIpv4HostAddress() method. It does not add value, so I removed it and just return the first one, or null if there is none. Null return is documented.
Signed-off-by: Mark Herwege <[email protected]>
| * @return list of broadcast addresses, empty list if no broadcast addresses found | ||
| */ | ||
| public static List<String> getAllBroadcastAddresses() { | ||
| List<String> broadcastAddresses = new LinkedList<String>(); |
There was a problem hiding this comment.
Hm, shouldn't the formatter replace new LinkedList<String>() be replaced by an Diamond Operator?
There was a problem hiding this comment.
Hm, okay, seems it is not activated by the ESH formatter.
| * @return list of broadcast addresses, empty list if no broadcast addresses found | ||
| */ | ||
| public static List<String> getAllBroadcastAddresses() { | ||
| List<String> broadcastAddresses = new LinkedList<String>(); |
There was a problem hiding this comment.
Hm, okay, seems it is not activated by the ESH formatter.
maggu2810
changed the title
|
core and core tests succeeded, the error is not related |
Signed-off-by: Mark Herwege [email protected]