Page 1 of 1

Look how checkstyle ruins good code!

PostPosted: 17 Mar 2012, 18:18
by Max mtg
There's a class of mine: /trunk/src/main/java/forge/quest/data/item/QuestItemType.java
http://svn.slightlymagic.net/websvn/log ... &peg=14779
Before checkstyle (r14717)

Code: Select all
public enum QuestItemType {
   
    SLEIGHT("Sleight", QuestItemPassive.class, QuestItemCondition.class),
    ESTATES("Estates", QuestItemEstates.class, QuestItemCondition.class),
    LUCKY_COIN("Lucky Coin", QuestItemPassive.class, QuestItemCondition.class),
    MAP("Map", QuestItemPassive.class, QuestItemCondition.class),
    ZEPPELIN("Zeppelin", QuestItemZeppelin.class, QuestItemCondition.class),
    ELIXIR_OF_LIFE("Elixir of Life", QuestItemElixir.class, QuestItemCondition.class),
    POUND_FLESH("Pound of Flesh", QuestItemPoundFlesh.class, QuestItemCondition.class);
   
   
    private final String saveFileKey;
After checkstyle (r14765)
Code: Select all
public enum QuestItemType {

    /** The SLEIGHT. */
    SLEIGHT("Sleight", QuestItemPassive.class, QuestItemCondition.class), /** The ESTATES. */
 ESTATES("Estates", QuestItemEstates.class,
            QuestItemCondition.class),
 /** The LUCK y_ coin. */
 LUCKY_COIN("Lucky Coin", QuestItemPassive.class, QuestItemCondition.class),
 /** The MAP. */
 MAP(
            "Map", QuestItemPassive.class, QuestItemCondition.class),
 /** The ZEPPELIN. */
 ZEPPELIN("Zeppelin", QuestItemZeppelin.class,
            QuestItemCondition.class),
 /** The ELIXI r_ o f_ life. */
 ELIXIR_OF_LIFE("Elixir of Life", QuestItemElixir.class, QuestItemCondition.class),
 /** The POUN d_ flesh. */
 POUND_FLESH(
            "Pound of Flesh", QuestItemPoundFlesh.class, QuestItemCondition.class);

    private final String saveFileKey;
Who still thinks that current checkstyle settings make code better?

Re: Look how checkstyle ruins good code!

PostPosted: 18 Mar 2012, 14:30
by Chris H.
I wonder if this was caused by a script and was not done by hand.

It does look strange.

Re: Look how checkstyle ruins good code!

PostPosted: 19 Mar 2012, 15:29
by jendave
I'll remove that setting. It obviously is not working.

Re: Look how checkstyle ruins good code!

PostPosted: 20 Mar 2012, 06:58
by Doublestrike
@Jendave - thanks for your efforts in putting in some standards. I enjoy having some guidelines, and a unified, clean code base with standardized formatting and without compiler flags is fantastic, especially with the large pool of developers we enjoy.

While you're digging around in there, could you have a look at the "file does not end with a newline" flag, quite a few files have this flag even though the file does end with a newline. Is there a way I can fix, or should the flag just be disabled?

Re: Look how checkstyle ruins good code!

PostPosted: 22 Mar 2012, 01:51
by SBeauchamp
Doublestrike wrote:While you're digging around in there, could you have a look at the "file does not end with a newline" flag, quite a few files have this flag even though the file does end with a newline. Is there a way I can fix, or should the flag just be disabled?
I had the same problem as you (I'm using Windows). I've replaced:

Code: Select all
<module name="NewlineAtEndOfFile"/>
with

Code: Select all
<module name="NewlineAtEndOfFile"><property name="lineSeparator" value="lf"/></module>
in the Checkfile parameter file, and it looks like it fixed the problem for me (even if it represents the Unix style newline). I'd be curious if that also does the trick on the other platforms. I'll commit the changes, let me know if it doesn't work properly!

Edit: More info here:
http://checkstyle.sourceforge.net/config_misc.html
http://checkstyle.2069334.n4.nabble.com/Checkstyle-on-Windows-td3831484.html

Re: Look how checkstyle ruins good code!

PostPosted: 22 Mar 2012, 10:13
by silly freak
by the way, eclipse has a setting for which line separator to use. I know you can set it on the file level, I'm not sure it's possible on the project level too. standardizing this might be a good idea.