search

peter-lawrey / Java-Chronicle

Java Indexed Record Chronicle
1.22k stars 193 forks source link

Sonar critical violations fixed #27

Closed neomatrix369 closed 11 years ago

neomatrix369 commented 11 years ago

Some pending violations related to language syntax mainly.

Only tests that failed is the below, which fails even before the changes were applied. Otherwise build and test went fine.

=== performing get test === Average get time including startup, bootstrap and shutdown, took 0.045 us average per key May 04, 2013 10:14:46 PM com.higherfrequencytrading.chronicle.tcp.InProcessChronicleSink readNextExcerpt INFO: Lost connection to localhost/127.0.0.1:12348 retrying java.nio.channels.AsynchronousCloseException Tests run: 5, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 20.164 sec <<< FAILURE!

Results :

Failed tests: testOverTCP(com.higherfrequencytrading.chronicle.impl.InProcessChronicleTest): expected:<17869> but was:<83405>

Tests in error: testOverTcp(com.higherfrequencytrading.chronicle.datamodel.MapWrapperTest): Address already in use

Tests run: 32, Failures: 1, Errors: 1, Skipped: 3

peter-lawrey commented 11 years ago

I am not convinced all the changes are desirable, but they all deserve more commenting as to why this is the case.

I will accept and review the changes first and perhaps comment some of them.

peter-lawrey commented 11 years ago

In one case, the try/catch has to catch Throwable otherwise the thread will die silently.

In other cases, some refactoring was needed.

Thank you for running this check, I think it helped improve the quality in many cases.

neomatrix369 commented 11 years ago

Thanks Peter, just made some of the aspect readable.

About Throwable it swallows both Errors and Exceptions, in the case of your library do you want to also handle Errors - if its an exceptional case then you might be right, but its not advisable.

peter-lawrey commented 11 years ago

The way it is written, the task/thread will log that it is dying and why before it dies. The alternative is to die silently which I believe is confusing.

This is the case because the task is submitted to an ExecutionService which will not log or report errors thrown by Runnable or Callable.