Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A few fixes for issues found by spotbugs #1278

Merged
merged 3 commits into from
Feb 27, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package htsjdk.samtools.cram.encoding.readfeatures;

import java.io.Serializable;
import java.util.Objects;

/**
* A substitution event captured in read coordinates. It is characterized by position in read, read base and reference base.
Expand Down Expand Up @@ -116,6 +117,14 @@ public boolean equals(final Object obj) {
return true;
}

@Override
public int hashCode() {
if (code == NO_CODE) {
return Objects.hash(position);
}
return Objects.hash(position, base, referenceBase, code);
jmthibault79 marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public String toString() {
return String.valueOf((char) operator) + '@' + position + '\\' + (char) base + (char) referenceBase;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.Objects;

public class CramCompressionRecord {
private static final int MULTI_FRAGMENT_FLAG = 0x1;
Expand Down Expand Up @@ -133,6 +134,17 @@ private boolean deepEquals(final Collection<?> c1, final Collection<?> c2) {
return (c1 == null || c1.isEmpty()) && (c2 == null || c2.isEmpty()) || c1 != null && c1.equals(c2);
}

@Override
jmthibault79 marked this conversation as resolved.
Show resolved Hide resolved
public int hashCode() {
int result = Objects.hash(alignmentStart, readLength, recordsToNextFragment, mappingQuality, flags, readName);
if (readFeatures != null && !readFeatures.isEmpty()) {
result = 31 * result + Objects.hash(readFeatures);
}
result = 31 * result + Arrays.hashCode(readBases);
result = 31 * result + Arrays.hashCode(qualityScores);
return result;
}

@Override
public String toString() {
final StringBuilder stringBuilder = new StringBuilder("[");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Scanner;
import java.util.regex.MatchResult;

Expand Down Expand Up @@ -135,6 +136,11 @@ public boolean equals(Object other) {
return true;
}

@Override
public int hashCode() {
return Objects.hash(sequenceEntries);
}

/**
* Parse the contents of an index file, caching the results internally.
* @param in InputStream to parse.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
* Note that this implementation is not synchronized. If multiple threads access an instance concurrently, it must be synchronized externally.
*/
public class AsyncBlockCompressedInputStream extends BlockCompressedInputStream {
private static final int READ_AHEAD_BUFFERS = (int)Math.ceil(Defaults.NON_ZERO_BUFFER_SIZE / BlockCompressedStreamConstants.MAX_COMPRESSED_BLOCK_SIZE);
private static final int READ_AHEAD_BUFFERS = (int)Math.ceil((double) Defaults.NON_ZERO_BUFFER_SIZE / BlockCompressedStreamConstants.MAX_COMPRESSED_BLOCK_SIZE);
private static final Executor threadpool = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors(),new ThreadFactory() {
@Override
public Thread newThread(Runnable r) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/htsjdk/samtools/util/CigarUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ static private void elementStraddlesClippedRead(List<CigarElement> newCigar, Cig
final CigarOperator op = c.getOperator();
int clipAmount = clippedBases;
if (op.consumesReadBases()){
if (op.consumesReferenceBases() & relativeClippedPosition > 0){
if (op.consumesReferenceBases() && relativeClippedPosition > 0){
newCigar.add(new CigarElement(relativeClippedPosition, op));
}
if (!op.consumesReferenceBases()){
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public final void close() {
try {
is.close();
} catch (IOException ex) {
new TribbleException("Failed to close PositionalBufferedStream", ex);
throw new TribbleException("Failed to close PositionalBufferedStream", ex);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import java.util.List;
import java.util.Map;
import java.util.HashMap;
import java.util.Objects;

public class GenotypeLikelihoods {
private final static int NUM_LIKELIHOODS_CACHE_N_ALLELES = 5;
Expand Down Expand Up @@ -158,6 +159,11 @@ public String getAsString() {
return Arrays.equals(getAsPLs(), that.getAsPLs());
}

@Override
public int hashCode() {
return Arrays.hashCode(getAsPLs());
}

//Return genotype likelihoods as an EnumMap with Genotypes as keys and likelihoods as values
//Returns null in case of missing likelihoods
public EnumMap<GenotypeType,Double> getAsMap(boolean normalizeFromLog10){
Expand Down
29 changes: 29 additions & 0 deletions src/test/java/htsjdk/samtools/cram/encoding/ReadFeaturesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@
import htsjdk.samtools.cram.encoding.readfeatures.Bases;
import htsjdk.samtools.cram.encoding.readfeatures.Scores;
import htsjdk.samtools.cram.encoding.readfeatures.SoftClip;
import htsjdk.samtools.cram.encoding.readfeatures.Substitution;
import org.testng.Assert;
import org.testng.annotations.Test;

import java.util.ArrayList;
import java.util.List;

public class ReadFeaturesTest extends HtsjdkTest {

// equals() was incorrect for these classes
Expand All @@ -25,4 +29,29 @@ public void faultyEquality() {
final SoftClip sc2 = new SoftClip(0, new byte[] {});
Assert.assertEquals(sc1, sc2);
}

@Test
public void testSubstitutionEqualsAndHashCodeAreConsistent() {
final List<Substitution> substitutions = new ArrayList<>();
for (int position : new int[] {0, 1}) {
for (byte base : new byte[] {(byte) -1, (byte) 'A'}) {
for (byte referenceBase : new byte[] {(byte) -1, (byte) 'C'}) {
for (byte code : new byte[] {Substitution.NO_CODE, (byte) 2}) {
jmthibault79 marked this conversation as resolved.
Show resolved Hide resolved
Substitution substitution = new Substitution(position, base, referenceBase);
substitution.setCode(code);
substitutions.add(substitution);
}
}
}
}

for (Substitution s1 : substitutions) {
for (Substitution s2 : substitutions) {
if (s1.equals(s2)) {
Assert.assertEquals(s1.hashCode(), s2. hashCode(),
String.format("Comparing %s (%s) and %s (%s)", s1, s1.getCode(), s2, s2.getCode()));
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package htsjdk.samtools.cram.structure;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import htsjdk.HtsjdkTest;
import htsjdk.samtools.SAMRecord;
import htsjdk.samtools.cram.encoding.readfeatures.*;
import org.testng.Assert;
import org.testng.annotations.Test;

import java.util.ArrayList;
import java.util.List;

/**
* Created by vadim on 28/09/2015.
Expand Down Expand Up @@ -85,4 +88,41 @@ public void test_isPlaced() {
r.alignmentStart = 15;
Assert.assertTrue(r.isPlaced());
}

@Test
public void testEqualsAndHashCodeAreConsistent() {
final List<CramCompressionRecord> records = new ArrayList<>();

for (int alignmentStart : new int[] {0, 1}) {
for (int readLength : new int[] {100, 101}) {
for (int flags : new int[] {0, 0x4}) {
for (List<ReadFeature> readFeatures : Lists.<List<ReadFeature>>newArrayList(null, new ArrayList<>())) {
for (String readName : new String[] {null, "r"}) {
for (byte[] readBases : new byte[][]{null, new byte[]{(byte) 'A', (byte) 'C'}}) {
for (byte[] qualityScores : new byte[][]{null, new byte[]{(byte) 1, (byte) 2}}) {
final CramCompressionRecord r = new CramCompressionRecord();
r.alignmentStart = alignmentStart;
r.readLength = readLength;
r.flags = flags;
r.readFeatures = readFeatures;
r.readName = readName;
r.readBases = readBases;
r.qualityScores = qualityScores;
records.add(r);
}
}
}
}
}
}
}

for (CramCompressionRecord r1 : records) {
for (CramCompressionRecord r2 : records) {
if (r1.equals(r2)) {
Assert.assertEquals(r1.hashCode(), r2.hashCode(), String.format("Comparing %s and %s", r1, r2));
}
}
}
}
}