responding to another round of comments

src/main/java/org/broadinstitute/hellbender/tools/spark/transforms/markduplicates/MarkDuplicatesSpark.java

@@ -1,6 +1,5 @@
 package org.broadinstitute.hellbender.tools.spark.transforms.markduplicates;
 
-import com.google.common.collect.Iterators;
 import htsjdk.samtools.SAMFileHeader;
 import htsjdk.samtools.metrics.MetricsFile;
 import org.apache.spark.Partitioner;
@@ -18,7 +17,6 @@
 import org.broadinstitute.hellbender.engine.filters.ReadFilter;
 import org.broadinstitute.hellbender.engine.filters.ReadFilterLibrary;
 import org.broadinstitute.hellbender.engine.spark.GATKSparkTool;
-import org.broadinstitute.hellbender.engine.spark.datasources.ReadsSparkSource;
 import org.broadinstitute.hellbender.exceptions.GATKException;
 import org.broadinstitute.hellbender.utils.Utils;
 import org.broadinstitute.hellbender.utils.read.GATKRead;
@@ -27,10 +25,9 @@
 import org.broadinstitute.hellbender.utils.read.markduplicates.GATKDuplicationMetrics;
 import org.broadinstitute.hellbender.utils.read.markduplicates.MarkDuplicatesScoringStrategy;
 import org.broadinstitute.hellbender.utils.read.markduplicates.SerializableOpticalDuplicatesFinder;
-import picard.sam.markduplicates.util.OpticalDuplicateFinder;
 import org.broadinstitute.hellbender.utils.spark.SparkUtils;
-import picard.sam.markduplicates.util.OpticalDuplicateFinder;
 import picard.cmdline.programgroups.ReadDataManipulationProgramGroup;
+import picard.sam.markduplicates.util.OpticalDuplicateFinder;
 import scala.Tuple2;
 
 import java.util.Collections;
@@ -113,9 +110,9 @@ public static JavaRDD<GATKRead> mark(final JavaRDD<GATKRead> reads, final SAMFil
                     .peek(read -> read.setIsDuplicate(false))
                     .peek(read -> {
                 // Handle reads that have been marked as non-duplicates (which also get tagged with optical duplicate summary statistics)
-                if( namesOfNonDuplicateReadsAndOpticalCounts.containsKey(read.getName())) {
+                if (namesOfNonDuplicateReadsAndOpticalCounts.containsKey(read.getName())) {
                     read.setIsDuplicate(false);
-                    if ( markUnmappedMates || !read.isUnmapped()) {
+                    if (markUnmappedMates || !read.isUnmapped()) {
                         int dupCount = namesOfNonDuplicateReadsAndOpticalCounts.replace(read.getName(), -1);
                         if (dupCount > -1) {
                             ((SAMRecordToGATKReadAdapter) read).setTransientAttribute(MarkDuplicatesSparkUtils.OPTICAL_DUPLICATE_TOTAL_ATTRIBUTE_NAME, dupCount);
@@ -125,8 +122,8 @@ public static JavaRDD<GATKRead> mark(final JavaRDD<GATKRead> reads, final SAMFil
                 } else if (ReadUtils.readAndMateAreUnmapped(read)) {
                     read.setIsDuplicate(false);
                     // Everything else is a duplicate
-                } else{
-                    if ( markUnmappedMates || !read.isUnmapped()) {
+                } else {
+                    if (markUnmappedMates || !read.isUnmapped()) {
                         read.setIsDuplicate(true);
                     } else {
                         read.setIsDuplicate(false);

src/main/java/org/broadinstitute/hellbender/utils/read/ReadUtils.java

@@ -316,16 +316,16 @@ public static String prettyPrintSequenceRecords( final SAMSequenceDictionary seq
     }
 
     /**
-     * @param read read to check
-     * @return true if the read is paired and has a mapped mate, otherwise false
+     * @param read read to query
+     * @return true if the read has a mate and that mate is mapped, otherwise false
      */
     public static boolean readHasMappedMate( final GATKRead read ) {
         return read.isPaired() && ! read.mateIsUnmapped();
     }
 
     /**
-     * @param read read to check
-     * @return true if the read is paired and has a mapped mate, otherwise false
+     * @param read read to query
+     * @return true if the read has a mate and that mate is mapped, otherwise false
      */
     public static boolean readHasMappedMate( final SAMRecord read ) {
         return read.getReadPairedFlag() && ! read.getMateUnmappedFlag();

src/main/java/org/broadinstitute/hellbender/utils/read/markduplicates/GATKDuplicationMetrics.java

@@ -34,18 +34,18 @@ public GATKDuplicationMetrics copy() {
     /**
      * Update metrics given a record or GATKRead
      */
-    public void updateMetrics(SAMRecord rec) {
+    public void updateMetrics(final SAMRecord rec) {
         update(rec.getReadUnmappedFlag(), rec.isSecondaryOrSupplementary(), ReadUtils.readHasMappedMate(rec), rec.getDuplicateReadFlag() );
     }
 
-    public void updateMetrics(GATKRead read) {
+    public void updateMetrics(final GATKRead read) {
         update(read.isUnmapped(), read.isSecondaryAlignment() || read.isSupplementaryAlignment(), ReadUtils.readHasMappedMate(read), read.isDuplicate() );
     }
 
-    private void update(boolean readUnmappedFlag, boolean secondaryOrSupplementary, boolean mappedMate, boolean isDuplicate) {
+    private void update(final boolean readUnmappedFlag, final boolean secondaryOrSupplementary, final boolean mappedMate, final boolean isDuplicate) {
         if (readUnmappedFlag) {
             ++this.UNMAPPED_READS;
-        } else if(secondaryOrSupplementary) {
+        } else if (secondaryOrSupplementary) {
             ++this.SECONDARY_OR_SUPPLEMENTARY_RDS;
         } else if (!mappedMate) {
             ++this.UNPAIRED_READS_EXAMINED;

src/main/java/org/broadinstitute/hellbender/utils/read/markduplicates/LibraryIdGenerator.java

@@ -46,7 +46,7 @@ public LibraryIdGenerator(final SAMFileHeader header) {
 
     public Histogram<Short> getOpticalDuplicatesByLibraryIdMap() { return this.opticalDuplicatesByLibraryId; }
 
-	public static String getReadGroupLibraryName(SAMReadGroupRecord readGroup) {
+	public static String getReadGroupLibraryName(final SAMReadGroupRecord readGroup) {
 		return Optional.ofNullable(readGroup.getLibrary())
 				.orElse(UNKNOWN_LIBRARY);
 	}

src/main/java/org/broadinstitute/hellbender/utils/test/testers/SamFileTester.java

@@ -24,7 +24,7 @@ public abstract class SamFileTester implements CommandLineProgramTester {
     private boolean deleteOnExit = true;
     protected final List<String> args = new ArrayList<>();
 
-    public SamFileTester(final int readLength, final boolean deleteOnExit, final int defaultChromosomeLength, final DuplicateScoringStrategy.ScoringStrategy duplicateScoringStrategy, final SAMFileHeader.SortOrder sortOrder, boolean recordsNeedSorting) {
+    public SamFileTester(final int readLength, final boolean deleteOnExit, final int defaultChromosomeLength, final DuplicateScoringStrategy.ScoringStrategy duplicateScoringStrategy, final SAMFileHeader.SortOrder sortOrder, final boolean recordsNeedSorting) {
         this.deleteOnExit = deleteOnExit;
         this.samRecordSetBuilder = new SAMRecordSetBuilder(recordsNeedSorting, sortOrder, true, defaultChromosomeLength, duplicateScoringStrategy);
         samRecordSetBuilder.setReadLength(readLength);

src/test/java/org/broadinstitute/hellbender/utils/test/testers/AbstractMarkDuplicatesTester.java

@@ -25,11 +25,11 @@ public abstract class AbstractMarkDuplicatesTester extends SamFileTester {
     final private File metricsFile;
     final DuplicationMetrics expectedMetrics;
 
-    public AbstractMarkDuplicatesTester(final ScoringStrategy duplicateScoringStrategy, SAMFileHeader.SortOrder sortOrder) {
+    public AbstractMarkDuplicatesTester(final ScoringStrategy duplicateScoringStrategy, final SAMFileHeader.SortOrder sortOrder) {
         this(duplicateScoringStrategy, sortOrder, true);
     }
 
-    public AbstractMarkDuplicatesTester(final ScoringStrategy duplicateScoringStrategy, SAMFileHeader.SortOrder sortOrder, boolean recordNeedSorting) {
+    public AbstractMarkDuplicatesTester(final ScoringStrategy duplicateScoringStrategy,  final SAMFileHeader.SortOrder sortOrder, final boolean recordNeedSorting) {
         super(50, true, SAMRecordSetBuilder.DEFAULT_CHROMOSOME_LENGTH, duplicateScoringStrategy, sortOrder, recordNeedSorting);
 
         expectedMetrics = new DuplicationMetrics();