-
Notifications
You must be signed in to change notification settings - Fork 428
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
Fix parameter meta data with whitespace characters #371
Fix parameter meta data with whitespace characters #371
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #371 +/- ##
============================================
- Coverage 40.16% 39.99% -0.17%
+ Complexity 1892 1883 -9
============================================
Files 107 107
Lines 24482 24511 +29
Branches 4038 4046 +8
============================================
- Hits 9833 9804 -29
- Misses 12815 12870 +55
- Partials 1834 1837 +3
Continue to review full report at Codecov.
|
@@ -330,7 +330,7 @@ private String escapeParse(StringTokenizer st, | |||
String fullName; | |||
nameFragment = firstToken; | |||
// skip spaces | |||
while (" ".equals(nameFragment) && st.hasMoreTokens()) { | |||
while ((0 == nameFragment.trim().length()) && st.hasMoreTokens()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a null check here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, if we look at the caller of the method, the variable will never be null since it's a token returned from StringTokenizer (instead of returning null, NoSuchElementException will be thrown if there are no more tokens in this tokenizer).
You are right, it's a good practice to always check for null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, currently firstToken
cannot be null
, but there might be more caller methods in the future :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are right :) Done
new PR equivalent to #351 which fixes #344