hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Drob <md...@apache.org>
Subject [DISCUSS] IntelliJ Code Analysis + Code Style
Date Mon, 19 Jun 2017 21:30:36 GMT
Hi Devs,

I recently ran the IntelliJ code analysis on the hbase project and saw that
it had some interesting suggestions.

A lot of them don't point to bugs, but really end up being more of a
stylistic issues. The ones I want to discuss specifically are the ones it
categorizes as java version issues. Here's the list:

Java 5:
* 'for' loop replaceable with 'foreach'
* Unnecessary unboxing
Java 7:
* Explicit type can be replaced with <>
* Identical 'catch' branch in 'try' statements
* Possible heap pollution from vararg type
Java 8:
* Anonymous type can be replaced with lambda
* Anonymous type can be replaced with method reference
* Lambda can be replaced with method reference
* Statement lambda can be replaced with expression lambda

I think I read somewhere once that 'foreach' is not great for garbage
collection because it creates an extra iterator, so we can maybe ignore
that one. And I personally prefer statement lambdas (with curly braces) to
expression lambdas (without) especially for long lines, so ignore that one
too. But maybe address the rest?

If we focus on only the Java 5/7 issues, then we can apply them to both
branch-1 and branch-2. Not sure how much value that provides, as I don't
have the historical context to estimate how long branch-1 will continue to
live once branch-2 is out.

The IDE can do the code changes automatically, but it will still be a lot
of time to review, hence the discussion before I cavalierly go and file an
issue and attach a patch. Also, I have no idea how to turn these into
automated checks once we fix any of them, and that seems kind of important.

IntelliJ has other checks that we can look at as well, but these were the
ones that caught my eye first as easiest to fix and least likely to false
positive.

Thoughts?

Mike

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message