[ 
https://issues.apache.org/jira/browse/SUREFIRE-1293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15560333#comment-15560333
 ] 

ASF GitHub Bot commented on SUREFIRE-1293:
------------------------------------------

Github user Tibor17 commented on a diff in the pull request:

    https://github.com/apache/maven-surefire/pull/127#discussion_r82527079
  
    --- Diff: 
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestSetRunListener.java
 ---
    @@ -76,7 +76,7 @@ public TestSetRunListener( ConsoleReporter 
consoleReporter, FileReporter fileRep
                                    StatisticsReporter statisticsReporter, 
boolean trimStackTrace,
                                    boolean isPlainFormat, boolean 
briefOrPlainFormat )
         {
    -        this.consoleReporter = consoleReporter;
    +        this.consoleReporter = consoleReporter != null ? consoleReporter : 
new NullConsoleReporter();
    --- End diff --
    
    All of these classes are in `surefire-common` project. Therefor it would be 
nice not to create a new object 
    `new NullConsoleReporter()` but instead keep it as static final constant in 
abstract Factory in surefire-common.
    This would mean that such null-check may be embedded in the factory, e.g. 
`buildConsoleReporter()`. I would say that the caller should pass non-null 
consistent object into this class `TestSetRunListener` via constructor and the 
called class should not make any guess about it (means if == null => guess hide 
the error). If we accepted such paradigm then all such occurrences in this PR 
would have to change it.


> Simplify org.apache.maven.plugin.surefire.report.TestSetRunListener by using 
> the null object pattern
> ----------------------------------------------------------------------------------------------------
>
>                 Key: SUREFIRE-1293
>                 URL: https://issues.apache.org/jira/browse/SUREFIRE-1293
>             Project: Maven Surefire
>          Issue Type: Improvement
>          Components: Maven Surefire Plugin
>            Reporter: Benedikt Ritter
>              Labels: github
>             Fix For: 2.19.2
>
>
> The class org.apache.maven.plugin.surefire.report.TestSetRunListener has a 
> lot of checks like this:
> {code:java}
> if( field != null )
> {
>     // do something with field
> }
> {code}
> This can be simplified by providing fallback implementations for the fields 
> being used by TestSetRunListener.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to