Skip to content

Commit 965364b

Browse files
Artem LabazinArtem Labazin
Artem Labazin
authored and
Artem Labazin
committed
Code style refactoring
1 parent a8241f9 commit 965364b

File tree

25 files changed

+106
-202
lines changed

25 files changed

+106
-202
lines changed

‎.codestyle/checkstyle.xml‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,6 @@ limitations under the License.
5252
</module>
5353

5454

55-
<!--http://checkstyle.sourceforge.net/config_sizes.html#FileLength-->
56-
<module name="FileLength">
57-
<property name="max" value="300"/>
58-
</module>
59-
60-
6155
<!--http://checkstyle.sourceforge.net/config_whitespace.html#FileTabCharacter-->
6256
<module name="FileTabCharacter">
6357
<property name="eachLine" value="true"/>
@@ -69,8 +63,11 @@ limitations under the License.
6963
<module name="Translation"/>
7064
<module name="UniqueProperties"/>
7165

66+
<module name="SuppressWarningsFilter" />
67+
7268

7369
<module name="TreeWalker">
70+
<module name="SuppressWarningsHolder" />
7471

7572
<!-- Checks for Javadoc comments. -->
7673
<!-- See http://checkstyle.sf.net/config_javadoc.html -->
@@ -318,9 +315,6 @@ limitations under the License.
318315
<!-- Checks for common coding problems -->
319316
<!-- See http://checkstyle.sf.net/config_coding.html -->
320317
<module name="CovariantEquals"/>
321-
<module name="DeclarationOrder">
322-
<property name="ignoreConstructors" value="true"/>
323-
</module>
324318
<module name="DefaultComesLast"/>
325319
<module name="EmptyStatement"/>
326320
<module name="EqualsAvoidNull"/>
@@ -359,7 +353,6 @@ limitations under the License.
359353
<module name="SuperClone"/>
360354
<module name="SuperFinalize"/>
361355
<module name="UnnecessaryParentheses"/>
362-
<module name="VariableDeclarationUsageDistance"/>
363356

364357

365358
<!-- Checks for class design -->
@@ -369,7 +362,6 @@ limitations under the License.
369362
</module>
370363
<module name="FinalClass"/>
371364
<module name="HideUtilityClassConstructor"/>
372-
<module name="MutableException"/>
373365
<module name="OneTopLevelClass"/>
374366

375367
<!-- Miscellaneous other checks. -->

‎.codestyle/findbugs.xml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,7 @@ limitations under the License.
2323
<Match>
2424
<Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"/>
2525
</Match>
26+
<Match>
27+
<Bug pattern="BC_UNCONFIRMED_CAST"/>
28+
</Match>
2629
</FindBugsFilter>

‎.codestyle/pmd.xml‎

Lines changed: 41 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -23,144 +23,59 @@ limitations under the License.
2323

2424
<description>PMD Rules</description>
2525

26-
<rule ref="rulesets/java/basic.xml">
26+
<rule ref="category/java/bestpractices.xml">
2727
<exclude name="AvoidUsingHardCodedIP" />
28+
<exclude name="UseVarargs" />
2829
</rule>
29-
30-
<rule ref="rulesets/java/braces.xml" />
31-
<rule ref="rulesets/java/clone.xml" />
32-
<rule ref="rulesets/java/unusedcode.xml"/>
33-
<rule ref="rulesets/java/finalizers.xml" />
34-
35-
36-
<rule ref="rulesets/java/imports.xml">
37-
<!-- Stupid rule... -->
30+
<rule ref="category/java/codestyle.xml">
31+
<exclude name="AbstractNaming" />
32+
<exclude name="AtLeastOneConstructor" />
33+
<exclude name="AvoidPrefixingMethodParameters" />
34+
<exclude name="CommentDefaultAccessModifier" />
35+
<exclude name="DefaultPackage" />
36+
<exclude name="FieldDeclarationsShouldBeAtStartOfClass" />
37+
<exclude name="LocalVariableCouldBeFinal" />
38+
<exclude name="LongVariable" />
39+
<exclude name="MethodArgumentCouldBeFinal" />
40+
<exclude name="OnlyOneReturn" />
41+
<exclude name="ShortClassName" />
42+
<exclude name="ShortMethodName" />
43+
<exclude name="ShortVariable" />
3844
<exclude name="TooManyStaticImports" />
45+
<exclude name="UselessParentheses" />
3946
</rule>
40-
41-
<rule ref="rulesets/java/codesize.xml" >
42-
<!-- See the explanation for TooManyMethods.-->
47+
<rule ref="category/java/design.xml">
48+
<exclude name="AvoidCatchingGenericException" />
49+
<exclude name="AvoidRethrowingException" />
50+
<exclude name="AvoidThrowingNullPointerException" />
51+
<exclude name="DataClass" />
52+
<exclude name="ExcessiveImports" />
53+
<exclude name="ExcessivePublicCount" />
54+
<exclude name="LawOfDemeter" />
55+
<exclude name="LoosePackageCoupling" />
4356
<exclude name="TooManyFields" />
44-
<!-- Design is very hard to measure by numbers like this.-->
45-
<!-- The number and quality of dependencies might be a better indicator, -->
46-
<!-- and that requires a different tool.-->
4757
<exclude name="TooManyMethods" />
48-
<!-- See the explanation for TooManyMethods.-->
49-
<exclude name="ExcessivePublicCount" />
50-
<!-- Needs better understanding and proper configuration-->
51-
<exclude name="StdCyclomaticComplexity" />
52-
<!-- Needs better understanding and proper configuration-->
53-
<exclude name="ModifiedCyclomaticComplexity" />
54-
<!-- See the explanation for TooManyMethods.-->
55-
<exclude name="ExcessiveParameterList" />
56-
<!-- Too abstract rule -->
57-
<exclude name="CyclomaticComplexity" />
58-
</rule>
5958

60-
<rule ref="rulesets/java/design.xml">
61-
<!-- Sometimes important to avoid "DOS attack" but not as a general rule-->
62-
<exclude name="AvoidSynchronizedAtMethodLevel" />
63-
<!-- It's just extra effort to write and read the final keyword-->
64-
<exclude name="ClassWithOnlyPrivateConstructorsShouldBeFinal" />
65-
<!-- Maybe good idea if PMD could exclude null checks from this-->
66-
<exclude name="ConfusingTernary" />
67-
<!-- Statistical analysis is prone to givin false positives. Potential god classes-->
68-
<!-- most probably violate something else, too.-->
69-
<!-- And dependency analysis tools also help here.-->
70-
<exclude name="GodClass" />
71-
<!-- Switch is sometimes very readable-->
72-
<exclude name="TooFewBranchesForASwitchStatement"/>
73-
<!-- A static utility is a static utility even if it masquerades as something-->
74-
<!-- else by using the Singleton pattern-->
75-
<exclude name="UseUtilityClass" />
76-
<!-- This is good advice, but since it's violated so much in this project-->
77-
<!-- and the problem is not big (especially with good syntax colouring),-->
78-
<!-- we'll keep this ignored for now.-->
79-
<exclude name="AvoidReassigningParameters" />
80-
<!-- Good idea almost always, but not quite.-->
81-
<!--<exclude name="ReturnEmptyArrayRatherThanNull" />-->
82-
<!-- Sometimes one step at a time makes clearer code.-->
83-
<!-- Debugging is also easier if the return value is in a variable.-->
84-
<exclude name="UnnecessaryLocalBeforeReturn" />
85-
<!-- There are valid reasons for passing arrays (making it nullable for example)-->
86-
<exclude name="UseVarargs" />
87-
<!-- TODO explain what false positives this gives-->
88-
<exclude name="MissingBreakInSwitch" />
89-
<!-- TODO enable when all findings have been fixed-->
90-
<exclude name="UseLocaleWithCaseConversions" />
91-
<!-- It gives a lot of warnings on 'equals' method, fixing would decrease readability-->
92-
<exclude name="SimplifyBooleanReturns"/>
93-
<!--Gives false positive-->
94-
<exclude name="FieldDeclarationsShouldBeAtStartOfClass"/>
95-
<!-- Good rule but in practice to often suppressed -->
96-
<exclude name="PreserveStackTrace"/>
97-
98-
<exclude name="AccessorMethodGeneration"/>
59+
<!-- remove it -->
60+
<exclude name="NcssCount" />
9961
</rule>
62+
<rule ref="category/java/documentation.xml">
63+
<exclude name="CommentSize" />
10064

101-
<rule ref="rulesets/java/migrating.xml">
102-
<!-- The annotation is not as readable and there is no way to state which-->
103-
<!-- line should throw the exception and with what message-->
104-
<exclude name="JUnitUseExpected" />
105-
<!-- Main code is not junit code-->
106-
<exclude name="JUnit4TestShouldUseTestAnnotation" />
65+
<!-- remove it -->
66+
<exclude name="CommentRequired" />
10767
</rule>
108-
109-
<rule ref="rulesets/java/naming.xml">
110-
<!-- Often good to start name with Abstract, but on the other hand this-->
111-
<!-- rule is a bit too much like Hungarian notation and I in interface names.-->
112-
<!-- Who cares if it's abstract or not when you are using it?-->
113-
<exclude name="AbstractNaming" />
114-
<!-- Opinion, for me a getter is not a command, it's a declarative-->
115-
<!-- data reference-->
68+
<rule ref="category/java/errorprone.xml">
11669
<exclude name="AvoidFieldNameMatchingMethodName" />
117-
<!-- Why should generics not be named properly, like all other things-->
118-
<!-- (well, except Windows filesystem roots)?-->
119-
<exclude name="GenericsNaming" />
120-
<!-- It can be long if it's the only way to make it good-->
121-
<exclude name="LongVariable" />
122-
<!-- It can be short if it's good-->
123-
<exclude name="ShortVariable" />
124-
<!-- TODO explain why.-->
125-
<exclude name="BooleanGetMethodName" />
126-
<!-- It can be short if it's good-->
127-
<exclude name="ShortClassName" />
128-
<!-- It can be short if it's good-->
129-
<exclude name="ShortMethodName" />
130-
</rule>
131-
132-
<rule ref="rulesets/java/optimizations.xml">
133-
<!-- Too many false hits. Optimization can't be done with static analysis.
134-
Besides, following this may encourage the antipattern of using too broad
135-
scope for stuff: -->
136-
<exclude name="AvoidInstantiatingObjectsInLoops" />
137-
<!-- Good principle but too verbose in practice: -->
138-
<exclude name="MethodArgumentCouldBeFinal" />
139-
<!-- Good principle and maybe sometimes even practical but not in this
140-
project: -->
141-
<exclude name="LocalVariableCouldBeFinal" />
142-
<exclude name="RedundantFieldInitializer" />
70+
<exclude name="AvoidFieldNameMatchingTypeName" />
71+
<exclude name="BeanMembersShouldSerialize" />
72+
<exclude name="DataflowAnomalyAnalysis" />
73+
<exclude name="DoNotCallSystemExit" />
74+
<exclude name="MissingBreakInSwitch" />
14375
</rule>
144-
145-
<rule ref="rulesets/java/strictexception.xml" >
146-
<!-- NPE communicates very cleary what is happening, it is not-->
147-
<!-- interesting who reports it (jvm or user code)-->
148-
<exclude name="AvoidThrowingNullPointerException" />
149-
<!-- TODO explain why-->
150-
<exclude name="AvoidCatchingGenericException" />
151-
<!-- TODO explain why-->
152-
<exclude name="AvoidThrowingRawExceptionTypes" />
153-
<!-- One valid case is to catch runtime, throw as such and after that-->
154-
<!-- catch Exception and wrap as runtime.-->
155-
<!-- Without the first all runtimes are unnecessarily wrapped.-->
156-
<exclude name="AvoidRethrowingException" />
157-
<!-- There are case which should throws generic exceptions -->
158-
<exclude name="SignatureDeclareThrowsException" />
76+
<rule ref="category/java/multithreading.xml">
15977
</rule>
160-
161-
<rule ref="rulesets/java/strings.xml" >
162-
<!-- Splitting to multiple lines is sometimes more readable.-->
163-
<!-- Besides, where's the proof that it affects performance?-->
164-
<exclude name="ConsecutiveAppendsShouldReuse" />
78+
<rule ref="category/java/performance.xml">
79+
<exclude name="AvoidUsingShortType" />
16580
</rule>
16681
</ruleset>

‎CHANGELOG.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
1111

1212
- Add more unit and integration tests.
1313

14+
## [1.0.0](https://github.com/appulse-projects/epmd-java/releases/tag/1.0.0) - 2018-03-16
15+
16+
Code style refactoring.
17+
18+
### Changed
19+
20+
- PMD, FindBugs and Checkstyle fixes;
21+
- Updated dependencies.
22+
1423
## [0.4.2](https://github.com/appulse-projects/epmd-java/releases/tag/0.4.2) - 2018-03-05
1524

1625
Removed lookup cache, which was full of bugs

‎client/README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ Include the dependency to your project's pom.xml file:
1414
<dependency>
1515
<groupId>io.appulse.epmd.java</groupId>
1616
<artifactId>client</artifactId>
17-
<version>0.4.2</version>
17+
<version>1.0.0</version>
1818
</dependency>
1919
...
2020
</dependencies>
@@ -23,7 +23,7 @@ Include the dependency to your project's pom.xml file:
2323
or Gradle:
2424

2525
```groovy
26-
compile 'io.appulse.epmd.java:client:0.4.2'
26+
compile 'io.appulse.epmd.java:client:1.0.0'
2727
```
2828

2929
### Create client

‎client/pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ limitations under the License.
2525
<parent>
2626
<groupId>io.appulse</groupId>
2727
<artifactId>epmd-java</artifactId>
28-
<version>0.4.2</version>
28+
<version>1.0.0</version>
2929
</parent>
3030

3131
<groupId>io.appulse.epmd.java</groupId>

‎client/src/main/java/io/appulse/epmd/java/client/EpmdClient.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ private static int getDefaultPort () {
190190
port = Integer.parseInt(value);
191191
}
192192
} catch (NumberFormatException | SecurityException ex) {
193-
throw new RuntimeException(ex);
193+
throw new IllegalStateException("Couldn't extract EPMD port", ex);
194194
}
195195
log.debug("Default EPMD port is: '{}'", port);
196196
return port;

‎client/src/test/java/io/appulse/epmd/java/client/LocalEpmdClientTest.java‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import static lombok.AccessLevel.PRIVATE;
2424
import static org.assertj.core.api.Assertions.assertThat;
2525
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
26-
import static io.appulse.epmd.java.core.model.response.EpmdDump.NodeDump.Status.OLD_OR_UNUSED;
2726

2827
import io.appulse.epmd.java.client.exception.EpmdConnectionException;
2928
import io.appulse.epmd.java.client.exception.EpmdRegistrationException;
@@ -189,26 +188,6 @@ public void dumpAll () {
189188
});
190189
}
191190

192-
// @Test
193-
public void stop () {
194-
client.register("stopped", 19028, R3_ERLANG, TCP, R6, R6);
195-
196-
client.stop("stopped");
197-
198-
val nodes = client.dumpAll();
199-
assertThat(nodes)
200-
.isNotEmpty();
201-
202-
val node = nodes.stream()
203-
.filter(it -> "stopped".equals(it.getName()))
204-
.findFirst()
205-
.orElse(null);
206-
207-
assertThat(node)
208-
.isNotNull()
209-
.extracting("status").isEqualTo(OLD_OR_UNUSED);
210-
}
211-
212191
@Test
213192
public void kill () {
214193
assertThat(client.kill())

‎core/pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ limitations under the License.
2525
<parent>
2626
<groupId>io.appulse</groupId>
2727
<artifactId>epmd-java</artifactId>
28-
<version>0.4.2</version>
28+
<version>1.0.0</version>
2929
</parent>
3030

3131
<groupId>io.appulse.epmd.java</groupId>

‎core/src/test/java/io/appulse/epmd/java/core/model/request/RegistrationTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@
1616

1717
package io.appulse.epmd.java.core.model.request;
1818

19-
import static io.appulse.epmd.java.core.model.Tag.ALIVE2_REQUEST;
20-
import static org.assertj.core.api.Assertions.assertThat;
2119
import static io.appulse.epmd.java.core.model.NodeType.R3_HIDDEN;
2220
import static io.appulse.epmd.java.core.model.Protocol.SCTP;
21+
import static io.appulse.epmd.java.core.model.Tag.ALIVE2_REQUEST;
2322
import static io.appulse.epmd.java.core.model.Version.R6;
23+
import static org.assertj.core.api.Assertions.assertThat;
2424

2525
import io.appulse.epmd.java.core.mapper.deserializer.MessageDeserializer;
2626
import io.appulse.epmd.java.core.mapper.serializer.MessageSerializer;

0 commit comments

Comments
 (0)