fixed throwing behavior in daemonConfig #179
Conversation
| @@ -101,6 +101,11 @@ private boolean setUDPAndTCPAddress(@Nullable String addr, boolean ignoreInvalid | |||
| boolean result = false; | |||
| try { | |||
| result = processAddress(addr); | |||
|
|
|||
| if (!result) { | |||
There was a problem hiding this comment.
Don't we need to check ignoreInvalid?
There was a problem hiding this comment.
I don't think so, because the exception will always be caught and then ignoreInvalid is checked in the catch block. If you think there's a more idiomatic way of doing this I'd be open to it though.
There was a problem hiding this comment.
Ah yeah didn't notice it - it's a bit weird to throw and immediately catch an exception. Outside of the try block having this with if (!result && !ignoreInvalid) throw might be more readable, but it's marginable at best.
There was a problem hiding this comment.
I actually just did a bigger refactor and made the processAddress method void, and just had it throw instead of return false. IMO this improved readability in both the setUDPandTCPAddress and processAddress methods, let me know what you think.
| @@ -101,6 +101,11 @@ private boolean setUDPAndTCPAddress(@Nullable String addr, boolean ignoreInvalid | |||
| boolean result = false; | |||
| try { | |||
| result = processAddress(addr); | |||
|
|
|||
| if (!result) { | |||
There was a problem hiding this comment.
Ah yeah didn't notice it - it's a bit weird to throw and immediately catch an exception. Outside of the try block having this with if (!result && !ignoreInvalid) throw might be more readable, but it's marginable at best.
Description of changes:
In
DaemonConfiguration, when setting a custom address for the daemon, the expected behavior is to throw an exception when an invalid address is passed to crash an application at startup if it's trying to configure an invalid address. The current behavior was to only crash if a somewhat invalid address, like0.0.0.1that's just missing a port, was passed. This fix ensures it crashes on arbitrary invalid strings as well.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.