Skip to content

Commit

Permalink
Improved consistency and formal correctness of all existing .equals m…
Browse files Browse the repository at this point in the history
…ethods to meet IntelliJ's convention.

* added null handling so equals methods return false for null argument
* replaced instanceof with getClass to force strict type equality
* added @OverRide for equals and hashCode where missing
* minor refactorings in equals methods to simplify and improve consistency
* added missing hashCode for ListMessage based on equals definition

Things that HAVE NOT changed:

* set of attributes used for equality checking
* hashCode calculation (except for added hashCode in ListMessage)
* correlation between equals and hashCode
* no new equals methods added
  • Loading branch information
Piotr Włodarek committed May 22, 2014
1 parent ff8d76c commit b32d0cc
Show file tree
Hide file tree
Showing 27 changed files with 160 additions and 140 deletions.
4 changes: 2 additions & 2 deletions core/src/main/java/com/google/bitcoin/core/Block.java
Original file line number Diff line number Diff line change
Expand Up @@ -805,8 +805,8 @@ public void verify() throws VerificationException {

@Override
public boolean equals(Object o) {
if (!(o instanceof Block))
return false;
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Block other = (Block) o;
return getHash().equals(other.getHash());
}
Expand Down
12 changes: 7 additions & 5 deletions core/src/main/java/com/google/bitcoin/core/BloomFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,13 @@ public boolean matchesAll() {
}

@Override
public boolean equals(Object other) {
return other instanceof BloomFilter &&
((BloomFilter) other).hashFuncs == this.hashFuncs &&
((BloomFilter) other).nTweak == this.nTweak &&
Arrays.equals(((BloomFilter) other).data, this.data);
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
BloomFilter other = (BloomFilter) o;
return hashFuncs == other.hashFuncs &&
nTweak == other.nTweak &&
Arrays.equals(data, other.data);
}

@Override
Expand Down
20 changes: 7 additions & 13 deletions core/src/main/java/com/google/bitcoin/core/DumpedPrivateKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,13 @@ public ECKey getKey() {
}

@Override
public boolean equals(Object other) {
// This odd construction is to avoid anti-symmetry of equality: where a.equals(b) != b.equals(a).
boolean result = false;
if (other instanceof VersionedChecksummedBytes) {
result = Arrays.equals(bytes, ((VersionedChecksummedBytes)other).bytes);
}
if (other instanceof DumpedPrivateKey) {
DumpedPrivateKey o = (DumpedPrivateKey) other;
result = Arrays.equals(bytes, o.bytes) &&
version == o.version &&
compressed == o.compressed;
}
return result;
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
DumpedPrivateKey other = (DumpedPrivateKey) o;
return Arrays.equals(bytes, other.bytes) &&
version == other.version &&
compressed == other.compressed;
}

@Override
Expand Down
16 changes: 5 additions & 11 deletions core/src/main/java/com/google/bitcoin/core/ECKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -402,13 +402,9 @@ protected ByteArrayOutputStream derByteStream() throws IOException {
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;

ECDSASignature signature = (ECDSASignature) o;

if (!r.equals(signature.r)) return false;
if (!s.equals(signature.s)) return false;

return true;
ECDSASignature other = (ECDSASignature) o;
return r.equals(other.r) &&
s.equals(other.s);
}

@Override
Expand Down Expand Up @@ -828,10 +824,8 @@ public void setCreationTimeSeconds(long newCreationTimeSeconds) {
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;

ECKey ecKey = (ECKey) o;

return Arrays.equals(pub, ecKey.pub);
ECKey other = (ECKey) o;
return Arrays.equals(pub, other.pub);
}

@Override
Expand Down
10 changes: 6 additions & 4 deletions core/src/main/java/com/google/bitcoin/core/GetBlocksMessage.java
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,13 @@ protected void bitcoinSerializeToStream(OutputStream stream) throws IOException

@Override
public boolean equals(Object o) {
if (o == null || o.getClass() != getClass()) return false;
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
GetBlocksMessage other = (GetBlocksMessage) o;
return (other.version == version &&
locator.size() == other.locator.size() && locator.containsAll(other.locator) &&
stopHash.equals(other.stopHash));
return version == other.version &&
locator.size() == other.locator.size() &&
locator.containsAll(other.locator) &&
stopHash.equals(other.stopHash);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,13 @@ public String toString() {
*/
@Override
public boolean equals(Object o) {
if (o == null || o.getClass() != getClass()) return false;
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
GetHeadersMessage other = (GetHeadersMessage) o;
return (other.version == version &&
locator.size() == other.locator.size() && locator.containsAll(other.locator) &&
stopHash.equals(other.stopHash));
return version == other.version &&
locator.size() == other.locator.size() &&
locator.containsAll(other.locator) &&
stopHash.equals(other.stopHash);
}

@Override
Expand Down
16 changes: 10 additions & 6 deletions core/src/main/java/com/google/bitcoin/core/InventoryItem.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,17 @@ public String toString() {
return type.toString() + ": " + hash;
}

public int hashCode() {
return hash.hashCode() + type.ordinal();
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
InventoryItem other = (InventoryItem) o;
return type == other.type &&
hash.equals(other.hash);
}

public boolean equals(Object o) {
return o instanceof InventoryItem &&
((InventoryItem)o).type == this.type &&
((InventoryItem)o).hash.equals(this.hash);
@Override
public int hashCode() {
return hash.hashCode() + type.ordinal();
}
}
13 changes: 9 additions & 4 deletions core/src/main/java/com/google/bitcoin/core/ListMessage.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ public abstract class ListMessage extends Message {

public static final long MAX_INVENTORY_ITEMS = 50000;


public ListMessage(NetworkParameters params, byte[] bytes) throws ProtocolException {
super(params, bytes, 0);
}
Expand All @@ -44,7 +43,6 @@ public ListMessage(NetworkParameters params, byte[] msg, boolean parseLazy, bool
super(params, msg, 0, parseLazy, parseRetain, length);
}


public ListMessage(NetworkParameters params) {
super(params);
items = new ArrayList<InventoryItem>();
Expand Down Expand Up @@ -124,7 +122,14 @@ public void bitcoinSerializeToStream(OutputStream stream) throws IOException {

@Override
public boolean equals(Object o) {
return o.getClass() == this.getClass() &&
((ListMessage)o).items.equals(this.items);
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ListMessage other = (ListMessage) o;
return items.equals(other.items);
}

@Override
public int hashCode() {
return items.hashCode();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,11 @@ public String getId() {
public abstract String getPaymentProtocolId();

@Override
public boolean equals(Object other) {
if (!(other instanceof NetworkParameters)) return false;
NetworkParameters o = (NetworkParameters) other;
return o.getId().equals(getId());
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
NetworkParameters other = (NetworkParameters) o;
return getId().equals(other.getId());
}

@Override
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/com/google/bitcoin/core/PeerAddress.java
Original file line number Diff line number Diff line change
Expand Up @@ -246,13 +246,14 @@ public String toString() {

@Override
public boolean equals(Object o) {
if (!(o instanceof PeerAddress)) return false;
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
PeerAddress other = (PeerAddress) o;
return other.addr.equals(addr) &&
other.port == port &&
other.services.equals(services) &&
other.time == time;
//FIXME including services and time could cause same peer to be added multiple times in collections
//TODO: including services and time could cause same peer to be added multiple times in collections
}

@Override
Expand Down
12 changes: 7 additions & 5 deletions core/src/main/java/com/google/bitcoin/core/RejectMessage.java
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,13 @@ public String toString() {

@Override
public boolean equals(Object o) {
return o instanceof RejectMessage &&
((RejectMessage) o).message.equals(message) &&
((RejectMessage) o).code.equals(code) &&
((RejectMessage) o).reason.equals(reason) &&
((RejectMessage) o).messageHash.equals(messageHash);
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
RejectMessage other = (RejectMessage) o;
return message.equals(other.message) &&
code.equals(other.code) &&
reason.equals(other.reason) &&
messageHash.equals(other.messageHash);
}

@Override
Expand Down
11 changes: 5 additions & 6 deletions core/src/main/java/com/google/bitcoin/core/Sha256Hash.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,12 @@ public static Sha256Hash hashFileContents(File f) throws IOException {
}
}

/**
* Returns true if the hashes are equal.
*/
@Override
public boolean equals(Object other) {
if (!(other instanceof Sha256Hash)) return false;
return Arrays.equals(bytes, ((Sha256Hash) other).bytes);
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Sha256Hash other = (Sha256Hash) o;
return Arrays.equals(bytes, other.bytes);
}

/**
Expand Down
12 changes: 7 additions & 5 deletions core/src/main/java/com/google/bitcoin/core/StoredBlock.java
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,13 @@ public boolean moreWorkThan(StoredBlock other) {
}

@Override
public boolean equals(Object other) {
if (!(other instanceof StoredBlock)) return false;
StoredBlock o = (StoredBlock) other;
return o.header.equals(header) && o.chainWork.equals(chainWork) && o.height == height;
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
StoredBlock other = (StoredBlock) o;
return header.equals(other.header) &&
chainWork.equals(other.chainWork) &&
height == other.height;
}

@Override
Expand All @@ -94,7 +97,6 @@ public int hashCode() {
return header.hashCode() ^ chainWork.hashCode() ^ height;
}


/**
* Creates a new StoredBlock, calculating the additional fields by adding to the values in this block.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,18 @@ public String toString() {
return String.format("Stored TxOut of %s (%s:%d)", Utils.bitcoinValueToFriendlyString(value), hash.toString(), index);
}

@Override
public int hashCode() {
return hash.hashCode() + (int)index;
}

@Override
public boolean equals(Object o) {
if (!(o instanceof StoredTransactionOutput)) return false;
return ((StoredTransactionOutput) o).getIndex() == this.getIndex() &&
((StoredTransactionOutput) o).getHash().equals(this.getHash());
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
StoredTransactionOutput other = (StoredTransactionOutput) o;
return getHash().equals(other.getHash()) &&
getIndex() == other.getIndex();
}

public void serializeToStream(OutputStream bos) throws IOException {
Expand All @@ -169,4 +173,4 @@ public void serializeToStream(OutputStream bos) throws IOException {
bos.write(0xFF & (height >> 16));
bos.write(0xFF & (height >> 24));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

/**
* Contains minimal data neccessary to disconnect/connect the transactions
* in the stored block at will. Can either store the full set of
* in the stored block at will. Can either store the full set of
* transactions (if the inputs for the block have not been tested to work)
* or the set of transaction outputs created/destroyed when the block is
* connected.
Expand Down Expand Up @@ -70,14 +70,18 @@ public List<Transaction> getTransactions() {
public Sha256Hash getHash() {
return blockHash;
}


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


@Override
public boolean equals(Object o) {
if (!(o instanceof StoredUndoableBlock)) return false;
return ((StoredUndoableBlock)o).getHash().equals(this.getHash());
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
StoredUndoableBlock other = (StoredUndoableBlock) o;
return getHash().equals(other.getHash());
}

@Override
Expand Down
10 changes: 5 additions & 5 deletions core/src/main/java/com/google/bitcoin/core/Transaction.java
Original file line number Diff line number Diff line change
Expand Up @@ -1176,11 +1176,11 @@ public synchronized boolean hasConfidence() {
}

@Override
public boolean equals(Object other) {
if (!(other instanceof Transaction)) return false;
Transaction t = (Transaction) other;

return t.getHash().equals(getHash());
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Transaction other = (Transaction) o;
return getHash().equals(other.getHash());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ public TransactionInput(NetworkParameters params, @Nullable Transaction parentTr
scriptBytes = EMPTY_ARRAY;
sequence = NO_SEQUENCE;
this.parentTransaction = parentTransaction;

length = 41;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,12 @@ private void writeObject(ObjectOutputStream out) throws IOException {
}

@Override
public boolean equals(Object other) {
if (!(other instanceof TransactionOutPoint)) return false;
TransactionOutPoint o = (TransactionOutPoint) other;
return o.getIndex() == getIndex() && o.getHash().equals(getHash());
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
TransactionOutPoint other = (TransactionOutPoint) o;
return getIndex() == other.getIndex() &&
getHash().equals(other.getHash());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,11 +352,11 @@ public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;

TransactionOutput output = (TransactionOutput) o;
TransactionOutput other = (TransactionOutput) o;

if (!Arrays.equals(scriptBytes, output.scriptBytes)) return false;
if (value != null ? !value.equals(output.value) : output.value != null) return false;
if (parentTransaction != null && parentTransaction != output.parentTransaction) return false;
if (!Arrays.equals(scriptBytes, other.scriptBytes)) return false;
if (value != null ? !value.equals(other.value) : other.value != null) return false;
if (parentTransaction != null && parentTransaction != other.parentTransaction) return false;

return true;
}
Expand Down

0 comments on commit b32d0cc

Please sign in to comment.