Update for existing Check: DeclarationOrder

[maxvetrenko]

Update existing Check to cover next rule: http://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s3.4.2.1-overloads-never-split

[romani]

new option should be provided - "boolean groupOverloadMethods"

[rdiachenko]

If true, avoids to split overload methods

Change to "If true, avoid splitting overload methods"

• int parentNode = aAST.getParent().getType();
  • final int parentType = parentNode;

Use parentType instead of parentNode as it was before

• if(mGroupOverloadMethods && (parentNode == TokenTypes.CLASS_DEF

  • | parentNode == TokenTypes.ENUM_DEF)) {
    1. Use short-circuit operators
    2. What about interfaces?

• Group overload methods private void groupOverloadMethods(DetailAST aObjectBlock)

Bad name and description. This method doesn't group anything it just checks for ...

DetailAST curToken

I don't understand this name

Integer methodNumber = methodCollector.get(methodName);

1. Bad name
2. Variable declaration is far away from its usage. It is used only in the inner if

if (methodCounter - methodNumber > 1) {

Avoid magic numbers

[rdiachenko]

Also update xdocs

[maxvetrenko]

Integer methodNumber = methodCollector.get(methodName); Bad name

can't make up any good name. Please, help.

[maxvetrenko]

The rest is done, check my commit.

[rdiachenko]

Check the grouping of overload.

Should be "Checks that overload methods are grouped together.

checkGroupingOverloadMethods

Should be "checkGroupingOfOverloadMethods" or "checkOverloadMethodsGrouping" or any other ideas..

HashMap <String, Integer> methodCollector = new HashMap<String, Integer>();

Program to interfaces instead of their implementations

Integer methodNumber

1. use primitives
2. change name to "previousIndex"

methodCounter

Change name to "currentIndex"

methodCollector

Change name to "methodIndexMap"

[maxvetrenko]

use primitives

Can't use primitive, because sometimes methodIndexMap.get(methodName) returns null. If it tries to assign "null" to int, it will be NPE

The rest is done.

[rdiachenko]

An example check's configuration grouping overload methods:

Change to "An example of a check's configuration for grouping overload methods"

Checks that overload methods are grouped together

• * should not be separated from each other.

Change to "Checks that if overload methods are grouped together they should not be separated from each other"

Overload methods never split

Change to "Overload methods should not be split"

public void testOverloadMethods() throws Exception

Change to "testOverloadMethodsGrouping"

[maxvetrenko]

Overload methods never split

Where did you find it? I don't understand..

[rdiachenko]

Where did you find it? I don't understand..

1. Open your commit (https://github.com/maxvetrenko/checkstyle/commit/81bce3b5b08ba9217d2c01779708877a3b321bc8)
2. Press Ctrl+F and insert "Overload methods never split" into the search field
3. Press Enter. The all usages of the text will be highlighted.

[romani]

looks good, but please make

1)

value="TRUE"

make it as value="true" in both files(java and xml)

2) from javadoc

• * public void notFoo() {} // Have to be after foo(int i, String s)

What is the reason of inner tag "" ?

3) update javadoc to have your name added to "@ author" tag. All Checks that you update should have your name as author.

[maxvetrenko]

1. Done
2. Done
3. Done

[romani]

ok, good, this update is ready for PullRequest(PR) - please do. Please make sure that UTs coverage is 100% or close.

[maxvetrenko]

Merged