Skip to content

Static code analysis task

rglszl edited this page Apr 20, 2017 · 17 revisions

Static code analysis

AndrásJánoky:

After running Statis Code Analysis programs (FindBugs, SonarLint) we found that the project has many smells, and also bugs that require attention. I fixed a few I was able to.

CompareNames.java
Not much to do here, only a single performance issue, an empty string was initialized as
String result = new String();
Taking up extra memory space instead of using the existing empty string.
String result = "";

ChebiCheck.java
One of the foreaches was not guarded against possible null pointer exceptions:
for (File f : root.listFiles()) {
files.add(f);
As root.listFiles() can return an empty File, which then produces a null pointer exception, so I embedded it in an if case.
if(root.listFiles() != null) {
File[] filePaths = root.listFiles();
if (filePaths != null) {
for (File f : filePaths) {
files.add(f);
}
}
}

Engine.java Most of it here was simple code duplication and unnecessary type declaration.
For example
private Collection services = new ArrayList();
changed to
private Collection services = new ArrayList<>();
One bug that might have caused problems in the future is this block:
catch (Error | Exception e) {
Errors should never be caught, because for some errors (out of memory error) the application must not try to recover.
catch (Exception e) should be used.

EntityBasedDiff.java Simple code duplication that had to be cleaned up. Merged switch cases together that do the same:
case1:
case2:
do something
And also merged some if-s using the || and && operators.

LászlóRigó:

Util.java:

It would be better to have NullObjects than returning and checking the null values in getMatchingSourceEntity()

We should write tests to improve the code coverage, there are functions that are not called (getStats, getMatchingSourceEntity)

Maybe we should change the StringBuffers to StringBuilders according to the issue created (#11)

The function createDeclaredAlignmentAlgorithms is considered horrible according to the comment above (line 65)

Clone this wiki locally