[bisq-network/bisq] (5/4) Test P2PDataStore expiration code and make testing time sensitive code easier. (#3568)

[Julian Knutsen]

julianknutsen commented on this pull request.



> @@ -41,25 +43,29 @@
     private int sequenceNumber;
     private byte[] signature;
     private long creationTimeStamp;
+    transient private final Clock clock;

After sleeping on it and coming back to it today, I don't like the idea of storing the clock in the object. Not necessarily because it is value-heavy, but because it is more testable and cleaner to pass in a new instance if needed instead of relying on the one that was generated at instantiation time

I've updated the stack with the patch that removes it in favor of passing it into the functions that need it.

After reviewing and refactoring this code over the last few weeks I am actually of the opinion that this needs more OO not less to stay maintainable and testable. Having one large integration test for everything isn't a great path forward. I've got a stack almost ready that leverages the inheritance that already exists to make the code a lot easier test and deduplicates a lot of the redundant helper functions.

I'll send it out tomorrow and whoever is interested can take a look and see if the pros outweigh the cons.

Thanks again for your feedback.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/3568#discussion_r343991655
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191107/2a6f76f6/attachment.html>