From 243014e184953904f74e81eaedcbe4b644c3fa58 Mon Sep 17 00:00:00 2001
From: Gerd Aschemann <gerd@aschemann.net>
Date: Mon, 16 Sep 2024 12:42:40 +0200
Subject: [PATCH] #343 Fix typos and clarify parameter names

to avoid conflicts and confusions with attributes.
---
 .../collect/PerRunResults.java                | 27 +++++++--------
 .../report/ConsoleReporter.java               | 10 +++---
 .../htmlsanitycheck/report/HtmlReporter.java  | 33 +++++++++----------
 .../report/JUnitXmlReporter.java              | 10 +++---
 .../htmlsanitycheck/report/Reporter.java      | 17 +++-------
 .../report/ReporterTest.groovy                |  6 ++--
 6 files changed, 45 insertions(+), 58 deletions(-)

diff --git a/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/collect/PerRunResults.java b/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/collect/PerRunResults.java
index e210a9a6..b3b66185 100644
--- a/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/collect/PerRunResults.java
+++ b/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/collect/PerRunResults.java
@@ -5,24 +5,24 @@
 import java.util.Objects;
 
 /**
- * Collects checking results of 1..n html files which are checked together in one "run".
+ * Collects checking results of 1..n HTML files which are checked together in one "run".
  * <p>
- * Can keep results spanning more than one file (e.g. unused-image-files).
+ * Can keep results spanning more than one file (e.g., unused-image-files).
  */
 public class PerRunResults implements RunResults {
 
-    public final static Long ILLEGAL_TIMER = -7353315L;
+    public static final Long ILLEGAL_TIMER = -7353315L;
     // magic number - also used in tests
-    private final static Long TIMER_STILL_RUNNING = 42L;
+    private static final Long TIMER_STILL_RUNNING = 42L;
     // unused images is the only check concerning all pages...
     private final List<SinglePageResults> resultsForAllPages;
-    // checking time is important - therefore we maintain a timer
+    // checking time is important - therefore, we maintain a timer
     private final Long startedCheckingTimeMillis;
     private Long finishedCheckingTimeMillis;
 
 
     /**
-     * constructs a container for all checking results, including:
+     * Constructs a container for all checking results, including
      * + checking results for every page (contained in @see SinglePageResults instances)
      * + results for the rather quirky @see UnusedImagesChecker
      * + a simple timer to validate the checks ran fast enough
@@ -36,7 +36,7 @@ public PerRunResults() {
     }
 
     /**
-     * return the List of results for the pages
+     * Return the List of results for the pages
      */
     @Override
     public List<SinglePageResults> getResultsForAllPages() {
@@ -44,14 +44,14 @@ public List<SinglePageResults> getResultsForAllPages() {
     }
 
     /**
-     * stop the checking timer
+     * Stop the checking timer
      */
     public void stopTimer() {
         finishedCheckingTimeMillis = System.currentTimeMillis();
     }
 
     /**
-     * query the timer
+     * Query the timer
      * if timer has not yet been stopped - return a crazy number
      */
     @Override
@@ -67,13 +67,12 @@ public Long checkingTookHowManyMillis() {
 
 
     /**
-     * adds one kind of checking results.
+     * Adds one kind of checking results.
      *
      * @param pageResults : checking results for a single HTML page
      */
     public void addPageResults(SinglePageResults pageResults) {
         assert resultsForAllPages != null;
-
         resultsForAllPages.add(pageResults);
     }
 
@@ -86,7 +85,7 @@ public int nrOfPagesChecked() {
     }
 
     /**
-     * returns the total number of checks performed on all pages
+     * Returns the total number of checks performed on all pages
      */
     @Override
     public int nrOfChecksPerformedOnAllPages() {
@@ -94,12 +93,10 @@ public int nrOfChecksPerformedOnAllPages() {
         return resultsForAllPages.stream()
                 .map(SinglePageResults::nrOfItemsCheckedOnPage)
                 .reduce(Integer::sum).orElse(0);
-
-
     }
 
     /**
-     * returns the total number of findings on all pages
+     * Returns the total number of findings on all pages
      */
     @Override
     public int nrOfFindingsOnAllPages() {
diff --git a/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/ConsoleReporter.java b/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/ConsoleReporter.java
index a5cda908..9c15b160 100644
--- a/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/ConsoleReporter.java
+++ b/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/ConsoleReporter.java
@@ -64,11 +64,11 @@ protected void reportOverallSummary() {
     }
 
     @Override
-    protected void reportPageSummary(SinglePageResults pageResult) {
-        printer.accept(String.format("Summary for file %s%n", pageResult.getPageFileName()));
-        printer.accept(String.format("page path  : %s", pageResult.getPageFilePath()));
-        printer.accept(String.format("page title : %s", pageResult.getPageTitle()));
-        printer.accept(String.format("page size  : %d bytes", pageResult.getPageSize()));
+    protected void reportPageSummary(SinglePageResults singlePageResults) {
+        printer.accept(String.format("Summary for file %s%n", singlePageResults.getPageFileName()));
+        printer.accept(String.format("page path  : %s", singlePageResults.getPageFilePath()));
+        printer.accept(String.format("page title : %s", singlePageResults.getPageTitle()));
+        printer.accept(String.format("page size  : %d bytes", singlePageResults.getPageSize()));
     }
 
     @Override
diff --git a/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/HtmlReporter.java b/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/HtmlReporter.java
index cca23366..49e4317d 100644
--- a/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/HtmlReporter.java
+++ b/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/HtmlReporter.java
@@ -63,9 +63,9 @@ public void initReport() {
     }
 
     /*
-    We need some static files next to the report html.. css, js and logo stuff.
+    We need some static files next to the report HTML, CSS, js and logo stuff.
 
-    Originally I posted this as a question to the gradle forum:
+    Originally I posted this as a question to the Gradle forum:
     http://forums.gradle.org/gradle/topics/-html-checking-plugin-how-to-copy-required-css-to-output-directory
 
     Answers were:
@@ -144,7 +144,7 @@ private String gitPropertiesAsComments() {
 
 
     /*
-     * copy css, javaScript and image/icon files to the html output directory,
+     * copy CSS, JavaScript and image/icon files to the HTML output directory,
      *
      */
     private void copyRequiredResourceFiles(String outputDirectoryPath, List<String> requiredResources) {
@@ -283,22 +283,22 @@ private static String infoBoxFooter() {
     }
 
     @Override
-    protected void reportPageSummary(SinglePageResults pageResult) {
-        String pageID = CreateLinkUtil.convertToLink(pageResult.getPageFileName());
+    protected void reportPageSummary(SinglePageResults singlePageResults) {
+        String pageID = CreateLinkUtil.convertToLink(singlePageResults.getPageFileName());
 
 
         write(String.format(
                 "%n%n<h1 id=\"%s\">Results for %s </h1>%n",
-                pageID, pageResult.getPageFileName()
+                pageID, singlePageResults.getPageFileName()
         ));
 
         write(String.format(
                 "location : <a href=\"%s\">%s</a> <p>%n",
-                pageResult.getPageFilePath(), pageResult.getPageFilePath()
+                singlePageResults.getPageFilePath(), singlePageResults.getPageFilePath()
         ));
 
-        int nrOfItemsChecked = pageResult.nrOfItemsCheckedOnPage();
-        int nrOfFindings = pageResult.nrOfFindingsOnPage();
+        int nrOfItemsChecked = singlePageResults.nrOfItemsCheckedOnPage();
+        int nrOfFindings = singlePageResults.nrOfFindingsOnPage();
 
         int percentageSuccessful = SummarizerUtil.percentSuccessful(
                 nrOfItemsChecked,
@@ -308,7 +308,7 @@ protected void reportPageSummary(SinglePageResults pageResult) {
 
         write(infoBoxHeader());
 
-        int pageSize = pageResult.getPageSize();
+        int pageSize = singlePageResults.getPageSize();
         String sizeUnit = (pageSize >= 1_000_000) ? "MByte" : "kByte";
 
         String pageSizeStr = String.valueOf(SummarizerUtil.threeDigitTwoDecimalPlaces(pageSize));
@@ -366,8 +366,8 @@ protected void reportSingleCheckDetails(SingleCheckResults checkResults) {
     }
 
     /**
-     * Tries to find a writable directory. First tries dirName,
-     * if that does not work takes User.dir as second choice.
+     * Tries to find a writable directory.
+     * First try `dirName`, if that does not work, take `User.dir` as second choice.
      *
      * @param dirName : e.g. /Users/aim42/projects/htmlsc/build/reports/htmlchecks
      * @return complete path to a writable file that does not currently exist.
@@ -400,11 +400,8 @@ protected void closeReport() {
             write("</body></html>\n");
             writer.flush();
 
-            String logMessage = String.format(
-                    "wrote report to %s%s%s%n",
-                    resultsOutputDir, File.separatorChar, REPORT_FILENAME
-            );
-            log.info(logMessage);
+            log.info("wrote a report to '{}{}{}'",
+                    resultsOutputDir, File.separatorChar, REPORT_FILENAME);
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }
@@ -424,7 +421,7 @@ protected void closeReport() {
 
  Unless required by applicable law or agreed to in writing, software
  distributed under the License is distributed on an
- "AS IS" BASIS,WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
  either express or implied.
  See the License for the specific language governing permissions and
  limitations under the License.
diff --git a/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/JUnitXmlReporter.java b/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/JUnitXmlReporter.java
index 253f0769..2da21b3a 100644
--- a/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/JUnitXmlReporter.java
+++ b/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/JUnitXmlReporter.java
@@ -53,8 +53,8 @@ protected void initReport() {
     }
 
     @Override
-    protected void reportPageSummary(SinglePageResults pageResult) {
-        String name = filenameOrTitleOrRandom(pageResult);
+    protected void reportPageSummary(SinglePageResults singlePageResults) {
+        String name = filenameOrTitleOrRandom(singlePageResults);
         String sanitizedPath = name.replaceAll("[^A-Za-z0-9_-]+", "_");
         File testOutputFile = new File(outputPath, "TEST-unit-html-" + sanitizedPath + ".xml");
 
@@ -64,13 +64,13 @@ protected void reportPageSummary(SinglePageResults pageResult) {
 
             writer.writeStartDocument();
             writer.writeStartElement("testsuite");
-            writer.writeAttribute("tests", String.valueOf(pageResult.nrOfItemsCheckedOnPage()));
-            writer.writeAttribute("failures", String.valueOf(pageResult.nrOfFindingsOnPage()));
+            writer.writeAttribute("tests", String.valueOf(singlePageResults.nrOfItemsCheckedOnPage()));
+            writer.writeAttribute("failures", String.valueOf(singlePageResults.nrOfFindingsOnPage()));
             writer.writeAttribute("errors", "0");
             writer.writeAttribute("time", "0");
             writer.writeAttribute("name", name);
 
-            for (SingleCheckResults singleCheckResult : pageResult.getSingleCheckResults()) {
+            for (SingleCheckResults singleCheckResult : singlePageResults.getSingleCheckResults()) {
                 writer.writeStartElement("testcase");
                 writer.writeAttribute("assertions", String.valueOf(singleCheckResult.getNrOfItemsChecked()));
                 writer.writeAttribute("time", "0");
diff --git a/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/Reporter.java b/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/Reporter.java
index f92c0e72..60eaefdd 100644
--- a/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/Reporter.java
+++ b/htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/Reporter.java
@@ -20,7 +20,7 @@ public abstract class Reporter {
      * create the reporter
      */
     protected Reporter() {
-        this.createdOnDate =  new SimpleDateFormat("dd. MMMM yyyy, HH:mm").format(new Date());
+        this.createdOnDate = new SimpleDateFormat("dd. MMMM yyyy, HH:mm").format(new Date());
         this.createdByHSCVersion = ProductInformation.VERSION;
     }
 
@@ -40,9 +40,8 @@ protected Reporter(PerRunResults runResults) {
     /**
      * add checking results for one page
      */
-    public List<SinglePageResults> addCheckingResultsForOnePage(SinglePageResults singlePageResults) {
+    public void addCheckingResultsForOnePage(SinglePageResults singlePageResults) {
         pageResults.add(singlePageResults);
-        return pageResults.stream().sorted(Comparator.comparing(SinglePageResults::getPageTitle)).collect(Collectors.toList());
     }
 
     /**
@@ -51,28 +50,22 @@ public List<SinglePageResults> addCheckingResultsForOnePage(SinglePageResults si
      * Uses template-method to delegate most concrete implementations to subclasses
      */
     public void reportFindings() {
-
         initReport();
-
         reportOverallSummary();
-
         reportAllPages();
-
         closeReport();
     }
 
     private void reportAllPages() {
-
         for (SinglePageResults pageResult : pageResults) {
             reportPageSummary(pageResult);// delegated to subclass
             reportPageDetails(pageResult);// implemented below
             reportPageFooter();// delegated to subclass
         }
-
     }
 
-    protected void reportPageDetails(SinglePageResults pageResults) {
-        for (SingleCheckResults resultForOneCheck : pageResults.getSingleCheckResults()) {
+    protected void reportPageDetails(SinglePageResults singlePageResults) {
+        for (SingleCheckResults resultForOneCheck : singlePageResults.getSingleCheckResults()) {
             reportSingleCheckSummary(resultForOneCheck);
             reportSingleCheckDetails(resultForOneCheck);
         }
@@ -97,7 +90,7 @@ protected void initReport() {
 
     protected abstract void reportOverallSummary();
 
-    protected abstract void reportPageSummary(SinglePageResults pageResult);
+    protected abstract void reportPageSummary(SinglePageResults singlePageResults);
 
     protected abstract void reportPageFooter();
 
diff --git a/htmlSanityCheck-core/src/test/groovy/org/aim42/htmlsanitycheck/report/ReporterTest.groovy b/htmlSanityCheck-core/src/test/groovy/org/aim42/htmlsanitycheck/report/ReporterTest.groovy
index 783ef16a..ba5fbe2d 100644
--- a/htmlSanityCheck-core/src/test/groovy/org/aim42/htmlsanitycheck/report/ReporterTest.groovy
+++ b/htmlSanityCheck-core/src/test/groovy/org/aim42/htmlsanitycheck/report/ReporterTest.groovy
@@ -25,7 +25,7 @@ class ReporterTest {
             }
 
             @Override
-            protected void reportPageSummary(SinglePageResults pageResult) {
+            protected void reportPageSummary(SinglePageResults singlePageResults) {
             }
 
             @Override
@@ -44,13 +44,13 @@ class ReporterTest {
     }
 
     @Test
-    public void testNothingReportedWithEmptyResults() {
+    void testNothingReportedWithEmptyResults() {
         SinglePageResults spr = new SinglePageResults()
         runResults.addPageResults(spr)
 
         reporter.reportFindings()
 
-        assertEquals("Empty ConsoleReporter has no check", 0, reporter.totalNrOfChecks())
+        assertEquals("Empty Reporter has no checks", 0, reporter.totalNrOfChecks())
         assertEquals("Empty Reporter shall have no findings", 0, reporter.totalNrOfFindings())
     }
 }