IndentationCheck. Correct indentation of anonymous classes.

124 views
Skip to first unread message

Alexey Zuy

unread,
Feb 22, 2015, 5:40:06 AM2/22/15
to checksty...@googlegroups.com
What is correct indentation for anonymous classes ?

Lets take a look at example:
addActionListener(new ActionListener() //method call starts here ...
{
   
public void actionPerformed(ActionEvent e) {
   
}
}); // ... and ends here
This 5-lines statement is actually just a method call with huge parameter definition. If it was looking like following:
addActionListener(ConcreteListenerFactory.getInstance().buildCustomListener(args));
we would probably line-wrapped it like this(assuming line-wrap indent is 4 spaces):
addActionListener(ConcreteListenerFactory
   
.getInstance() //4
   
.buildCustomListener(args)); //4
This clearly indicates that creation of ActionListener is nested within method call. Why we should not line-wrap anonymous class definition in the same way? e.g.:
addActionListener(new ActionListener()
   
{ //4
       
public void actionPerformed(ActionEvent e) {
       
}
   
});//4
Line-wrapping anonymous classes this way clearly indicates that opening curly brace is not just unnamed statement list, like in following example:
addActionListener(new ActionListenerForButton());
{
    doSmth
(list);
   
// ...
}


This thoughts lead us to the following: Anounymous class`s bodies should be indented as line-wrapping case.

On the other hand, pupular IDEs(Eclipse, NetBeans, IntellijIdea) format curly braces to be exactly under method call start without any indentation. In Guava library anonymous classes indented in the same way(link).

So, which indentation sheme is more correct? Which one should be forced by Check?


Alexey Zuy

unread,
Feb 23, 2015, 2:19:46 PM2/23/15
to checksty...@googlegroups.com
Why this test input is considered correct ?
https://github.com/checkstyle/checkstyle/blob/master/src/test/resources/com/puppycrawl/tools/checkstyle/indentation/IndentationCorrectIfAndParameterInput.java

For example, I expect "if" condition on line 17 to be indented this way(assuming this input intended for "basicOffset" = 2, "lineWrappingIndent" = 4):
if (conditionFirst("Loooooooooooooooooong", new // indent:4 ; exp:4; ok
   
SecondClassWithVeryVeryVeryLongName("Loooooooooooooooooog"). // indent:8 ; exp:8; ok
        getInteger
(new FooIfClass(), "Loooooooooooooooooog"), // indent:12 ; exp:12; ok
   
new InnerClassFoo())) {} // indent:4 ; exp:4; ok

"conditionFirst" call has several parameters which does not fit to 1 line. "SecondClassWithVeryVeryVeryLongName" is line wrapped, so its indent must be +4 relatively to "if" statement.
"getInteger" is called form "SecondClassWithVeryVeryVeryLongName" instance but situated new line, so it is nested line wrapping, so it must be indented +4 relatively to "SecondClassWithVeryVeryVeryLongName("Loooooooooooooooooog")"
"new InnerClassFoo()" is third parameter of "conditionFirst" so it is line wrapped relatively to it.

Am i right ?

Roman Ivanov

unread,
Feb 27, 2015, 12:12:47 AM2/27/15
to Alexey Zuy, checksty...@googlegroups.com
Hi Alexey,

Which one should be forced by Check?

We should follow all IDEs, indentation level of "{" and "}" should be the same.

Why this test input is considered correct ?

During previous bug fixing wave in that Check we decided to no touch that behavior, indentation is not correct but it should be fixed by https://github.com/checkstyle/checkstyle/issues/270 that is already assigned to you.
I propose to not change that logic till we come to that issue.

new InnerClassFoo())) {} // indent:4 ; exp:4; ok

No that should be 8, we can not be on the same indentation as IF, 

thanks,
Roman Ivanov
Reply all
Reply to author
Forward
0 new messages