Add __eq__ and __hash__ methods for LedgerAccount #51
Conversation
| @@ -180,6 +180,13 @@ def __init__(self, path, address): | |||
| def __repr__(self): | |||
| return f"<ledgereth.objects.LedgerAccount {self.address}>" | |||
|
|
|||
| def __eq__(self, other): | |||
| if isinstance(other, LedgerAccount): | |||
| return self.path == other.path and self.address == other.address | |||
There was a problem hiding this comment.
question: Why compare path here? The odds of an address collision with a different path are all but impossible.
There was a problem hiding this comment.
I agree with you when LedgerAccount is calculated from Ledger, but the class itself let you create different instances with the same address and different path, this instances shouldn't be equal between them.
There was a problem hiding this comment.
Have you run into this IRL? If so, under what circumstances?
Not asking for any changes, just trying to understand this so I don't introduce a regression unintentionally down the road.
There was a problem hiding this comment.
I haven't faced this scenario in real life. As far as I know, this could occur only if two ledgers are connected simultaneously, which isn't possible with the current implementation or someone manually initializes the class.
There was a problem hiding this comment.
Thinking about it, two ledgers shouldn't return the same address. I think the unique situation where this could happen is instantiating manually.
Done :) |
|
Thanks for your contribution. I'll probably have a release tonight. |
Closes #50
This PR defines
__eq__and__hash__methods onLedgerAccountclass to makes possible that instances ofLedgerAccountwith the samepathandaddresswere evaluated as equals.