[ 
https://issues.apache.org/jira/browse/MAHOUT-913?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13162333#comment-13162333
 ] 

Sean Owen commented on MAHOUT-913:
----------------------------------

I'm not suggesting standardizing on a tool, no. I do think these things are 
easier, since they are more automatic, with IntelliJ than Eclipse. It was a 
suggestion to lower the effort barrier, which could contribute to the issue. I 
actually don't like asking people to do more work, hopefully this is the 
opposite of that!

We may just agree to disagree on private static methods; I think a non-instance 
method should be declared as such, and I don't think it's quite trivial. 
Consensus rules though, but I thought there was mostly agreement on this 
particular one.

Formatting is most trivial and doesn't matter per se. I think it matters in 
that you probably have to be good at dealing with trivial issues before you can 
move up the stack, and there are most definitely less trivial issues I'd like 
to start talking about. If this is "level 0" (formatting) and "level 1" 
(redundant declarations), I'd like to start fixing "level 2" (wrong clone() or 
equals()/hashCode() pairs) and "level 3" (unencapsulated fields, polymorphic 
methods in constructors). Those are more real problems, and if you flip on 
inspections you'll see there are loads of them.
                
> Style changes / discussion
> --------------------------
>
>                 Key: MAHOUT-913
>                 URL: https://issues.apache.org/jira/browse/MAHOUT-913
>             Project: Mahout
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Sean Owen
>            Assignee: Sean Owen
>            Priority: Minor
>             Fix For: 0.6
>
>         Attachments: MAHOUT-913.patch
>
>
> Guys I've still been seeing code committed that doesn't match standard Java 
> style or a reasonable policy I can imagine. I wanted to talk about it since 
> I've just been silently changing it and that is not ideal.
> This should be easy to get right, as automated tools exist to check and fix 
> this. I recommend IntelliJ's free Community edition. Flip on even basic 
> inspections. A hundred things will jump out (that are already jumping out at 
> me). Most are automatically fixable. 
> I think that standardized, readable code invites attention, work and care: it 
> feels like something you want to improve, and don't want to hack up.
> I think it helps attract committers. Strong engineering organizations 
> wouldn't let basic style problems in the codebase, just by using automated 
> checks. Code reviews don't begin otherwise, and then reviews focus on real 
> issues like design. We can make a basic effort to approach that level of 
> quality. Otherwise, people who are used to a higher standard won't be 
> inclined to participate in the project, and will just fork.
> I think it's a prerequisite to fixing real design issues, TODOs, correctness 
> problems (cloning for instance), and refactorings. This code is not near that 
> point, and won't get there at this rate. 
> Personally it makes we want to only support anything I've written, and write 
> any "next generation" recommender system in a new and separate venture. And 
> I'm a friendly, and maybe not the only one! So would be great to keep some 
> focus on quality and design.
> Here's a patch showing all the changes I've picked up and made with the IDE 
> -- *just* basic style issues, and just since the last 2 weeks. The issues 
> are, among others:
>       ⁃       Empty javadoc
>       ⁃       Redundant javadoc ("@param foo the foo")
>       ⁃       Missing copyright headers
>       ⁃       Copyright headers not at top of file (sometimes after imports!)
>       ⁃       Very long lines (>> 120 chars)
>       ⁃       "throws Exception" not on main() or test method
>       ⁃       "transient" fields -- should never be used for us
>       ⁃       Missing @Override
>       ⁃       Using new Random()
>       ⁃       Redundant boolean expressions like "foo == true"
>       ⁃       Unused variables and parameters
>       ⁃       Unused imports
>       ⁃       Loops and conditionals without braces
>       ⁃       Weird literals ("1d")

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira


Reply via email to