Page History

2.1  Grammar and style

These are small things that are not caught by the automated style checkers.

• Does a variable need a better name?
• Should this be a keyword argument?
• In a PR, maintain the existing style of the file.
• Don’t combine code changes with lots of edits of whitespace or comments; it makes code review too difficult. It’s okay to fix an occasional comment or indenting, but if wholesale comment or whitespace changes are needed, make them a separate PR.
• Use the checkstyle plugin in Maven to verify that your PR conforms to our style

2.2  Code Style

• Follow the Google Java Style Guide outlined here: https://google.github.io/styleguide/javaguide.html

2.2.1 IntelliJ IDEA Checkstyle Warnings

Checkstyle: WhitespaceAround: 'if' is not followed by whitespace. Empty blocks may only be represented as {} when not part of a multi-block statement

2.3  Coding Standards

• Implementation matches what the documentation says  

• Logger name is effectively the result of Class.getName() 

• Class & member access - as restricted as it can be (subject to testing requirements)  

• Appropriate NullPointerException and IllegalArgumentException argument checks  

• Asserts - verify they should always be true  
• Look for accidental propagation of exceptions  
• Look for unanticipated runtime exceptions  
• Try-finally used as necessary to restore consistent state  

• Logging levels conform to Log4j levels 

• Possible deadlocks - look for inconsistent locking order  
• Race conditions - look for missing or inadequate synchronization  
• Consistent synchronization - always locking the same object(s)  
• Look for synchronization or documentation saying there's no synchronization  
• Look for possible performance problems  
• Look at boundary conditions for problems  
• Configuration entries are retrieved/set via setter/getter methods  
• Implementation details do NOT leak into interfaces  
• Variables and arguments should be interfaces where possible  

• If equals is overridden then hashCode is overridden (and vice versa)  

• Objects are checked (instanceof) for appropriate type before casting (use generics if possible)  

• Public API changes have been publicly discussed  
• Use of static member variables should be used with caution especially in Map/reduce tasks due to the JVM reuse feature 

2.4 Documentation

Code-Level Documentation

• • Self-documenting code (variable, method, class) has a clear semantic name
  • Accurate, sufficient for developers to code against
  • Follows standard Javadoc conventions
  • Loggers and logging levels covered if they do not follow our conventions (see below)
  • System properties, configuration options, and resources covered
  • Illegal arguments are properly documented as appropriate
  • Package and overview Javadoc are updated as appropriate
  • Javadoc comments are mandatory for all public APIs
  • Generate Javadocs for release builds
    
    

Feature-level documentation -  should be version controlled in github in README files.

• • Accurate description of the feature
  • Sample configuration and deployment options
  • Sample usage scenarios 
    
    

High-Level Design documentation - architecture description and diagrams should be a part of a wiki entry.

• • Provide diagrams/charts where appropriate.  Visuals are always welcome
  • Provide purpose of the feature/module and why it exists within the project
  • Describe system flows through the feature/module where appropriate
  • Describe how the feature/module interfaces with the rest of the system
  • Describe appropriate usages scenarios and use cases
    
    

Tutorials - system-level tutorials and use cases should also be kept as wiki entries.

• • Add to the Metron reference application documentation for each additional major feature
  • If appropriate, publish a tutorials blog on the Wiki to highlight usage scenarios and apply them to the real world use cases

2.5  Tests

• Unit tests exist for bug fixes and new features, or a rationale is given in JIRA for why there is no test 
•  Unit tests do not write any temporary files to /tmp (instead, the tests should write to the location specified by the test.build.data system property)