[GitHub] [maven-checkstyle-plugin] bmhm commented on a change in pull request #17: [MCHECKSTYLE-385] rework code to use a Violation.java class.

[GitBox <gi...@apache.org>]

bmhm commented on a change in pull request #17: [MCHECKSTYLE-385] rework code to use a Violation.java class.
URL: https://github.com/apache/maven-checkstyle-plugin/pull/17#discussion_r357109035
 
 

 ##########
 File path: src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleViolationCheckMojo.java
 ##########
 @@ -605,70 +610,118 @@ private void checkDeprecatedParameterUsage( Object parameter, String name, Strin
         }
     }
 
-    private int countViolations( XmlPullParser xpp )
+    private List<Violation> getViolations( XmlPullParser xpp )
         throws XmlPullParserException, IOException
     {
-        int count = 0;
-        int ignoreCount = 0;
-        List<RuleUtil.Matcher> ignores = violationIgnore == null ? Collections.<RuleUtil.Matcher>emptyList()
-                        : RuleUtil.parseMatchers( violationIgnore.split( "," ) );
+        List<Violation> violations = new ArrayList<>();
 
         String basedir = project.getBasedir().getAbsolutePath();
         String file = "";
+
         for ( int eventType = xpp.getEventType(); eventType != XmlPullParser.END_DOCUMENT; eventType = xpp.next() )
         {
             if ( eventType != XmlPullParser.START_TAG )
             {
                 continue;
             }
-            else if ( "file".equals( xpp.getName() ) )
+
+            if ( "file".equals( xpp.getName() ) )
 
 Review comment:
   Well if you really wanted to, I can rework this. I do only see disadvantages and probably most ppl will read `if (condition) continue;` as a guard statement:
   
   https://refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html
   
   Please also note that else if will make refactoring more complex, i.e. if adding code. Without the ifs, we could run into two `if`blocks if we wanted to. Using only  `if … else if`, we do not have this option and need to refactor for guard statements anyway.
   
   The docs did not say anything about "do not use guard statements". I can put them back in, if you like.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services