-
-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SpotBugs analysis: Possible NPE dereference in getPort() #202
Comments
I see that a default port would be useful. I don't see what the other changes are good for. Exporting the parameter Strings to constants will change exactly nothing as the constants will be inlined during compile time. The BusAddress class is designed to be used for any transport, so changing that one may break existing transports including custom transport I don't know of. It may be possible to allow better encapsulation in the next major version but I don't see a real issue here. |
Constants have many advantages for developers:
Encapsulation is a good idea, generally. To each their own though. :-) Thanks for fixing the NPEs! |
Yes it prevents typos, but as these few Strings are only used internally in TcpBusAddress, nobody needs to type it anywhere. Anyway, I updated the code once more to use better encapsulation and deprecated the getParameter() method. |
From SpotBugs:
The problem appears to be in the following code:
Calling
getParameters()
is a minor violation of encapsulation (because it leaks API details about how parameters are stored). Since all*BusAddress.java
classes extend fromBusAddress
, consider exposingget*Parameter()
andsetParameter()
methods (and the like) that hide the implementation details about how the parameter properties are stored. Consider:From there, it may be beneficial to force a default, such as:
Of course, if a default port isn't suitable, then maybe an
Optional
would work?This makes the code a little more
null
-hostile while addressing the potential NPE reported by SpotBugs and eliminating the duplicate/redundant calls togetParameters()
.The text was updated successfully, but these errors were encountered: