New NumericLiteralNeedUnderscore check: Issue #187

19 views
Skip to first unread message

Charlie Pai

unread,
Aug 25, 2015, 7:43:58 PM8/25/15
to Sevntu Checkstyle

Michal Kordas

unread,
Aug 26, 2015, 6:03:32 AM8/26/15
to sevntu-c...@googlegroups.com

Hi,

I believe we should first of all rename the check - should be NumericLiteralNeedsUnderscore.

Best,
Michal

26-08-2015 01:44 użytkownik "Charlie Pai" <mail4ch...@gmail.com> napisał:

--
You received this message because you are subscribed to the Google Groups "Sevntu Checkstyle" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sevntu-checkst...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Charlie Pai

unread,
Sep 17, 2015, 5:54:40 PM9/17/15
to Sevntu Checkstyle

Charlie Pai

unread,
Oct 19, 2015, 3:38:14 AM10/19/15
to Sevntu Checkstyle
Code review from Roman:
1) Show both valid and failure cases at the same time in Javadoc examples
2) Move examples in Javadoc to an example section
3) Describe default parameters
4) Make examples of non-default parameters

5) Change the name of the term "token" since it is used for visitToken and passesCheck

6) Single return point for getNumericType

7) Compiled Pattern for Regexp in generateStrippedTokens

8) Throw IllegalStateException in generateStrippedTokens

9) Make removePrePostfixByType, removePrefix, and removeLetterPostfix static

Also added a new test for floats to cover a case I forgot.


Roman Ivanov

unread,
Oct 23, 2015, 8:23:58 AM10/23/15
to sevntu-c...@googlegroups.com
Hi Charlie,


1)
> The minimum number of symbols in a decimal literal before
 
The minimum number of symbols in a decimal(long, float, double) literal before 

2)
> 
/**
* Sets how many characters in a literal there must be before it checks for an underscore
*/
public void setMinDecimalSymbolLen(int len)


please use @param and @return for all methods, that is requirement of main projects, just to avoid extra fixes during transition to main project.

3)
private boolean passesCheck(String rawLiteral)
{
int minCheckingLength = minSymbolsBeforeChecking(rawLiteral);
int symbolsUntilUnderscore = maxSymbolsUntilUnderscore(rawLiteral);
String[] numericSegments = getNumericSegments(rawLiteral);

Please make declaration of method base on first usage in code, so passesCheck, minSymbolsBeforeChecking, maxSymbolsUntilUnderscore,  getNumericSegments .

4)
Please make "2" as constant,  you use it in 3 places and 2 methods.

5)
private String[] getNumericSegments(String rawLiteral)
{
Type type = getNumericType(rawLiteral);

Please move "Type type = getNumericType(rawLiteral);" to upper level in this and other methods.

6)
I updated name of issue " new Check: NumericLiteralNeedUnderscoreCheck #187 ", please make commit message as "Issue #187: new Check - NumericLiteralNeedUnderscoreCheck". This will ease search for a commit in future.

7)

please look at https://github.com/applepi07/sevntu.checkstyle/commit/ff4fdd2c3c8002dfec38f1ad56e79756cbcb056d for set of files that need to be updated to let use your check in all extensions (eclipse, idea, sonar)


As all these minors from above resolved, I will merge your contribution and do release right-away. Thanks for your help.

thanks,
Roman Ivanov
​

Charlie Pai

unread,
Nov 8, 2015, 4:24:53 AM11/8/15
to Sevntu Checkstyle
1) Done
2) Done
3) Done
4) Done
5) Done except for getNumericSegments. I don't like having the function being able to return a bad value if a bad type parameter is passed in.
6) Done
7) Done

https://github.com/sevntu-checkstyle/sevntu.checkstyle/pull/390
Reply all
Reply to author
Forward
0 new messages