Hi,
thanks for bringing them up. I agree with you that we should remove them from the code and better open a ticket for then. I added them as it is an easy and direct way to point the reviewer to the point. One addition to the UUID generation: I get your point (never thought of this). Another idea woudl be to use a static atomic long or something as counter. Perhaps we can even skip all of this and stay with the current euqality check. But I think in situations where on sends several RequestItems in a request it could lead to problems, couldnt it? But lets do a jira for this and have the discussion there? Best Julian ________________________________ Von: Christofer Dutz <christofer.d...@c-ware.de> Gesendet: Donnerstag, 2. August 2018 10:46:58 An: dev@plc4x.apache.org Betreff: Feedback to the TODO items of Julians first Pull Request Hi, for the sake of completeness I am bringing aggregated feedback I also added on the pull request here, so it’s archived. I would probably remove the TODO items from the code, as I think these should rather be discussions on the list and/or Jira issues and we shouldn’t be having discussions in the code (just my opinion) * Using UUID for equality checks: I would hesitate using these, as generating a UUID consumes a lot of CPU power and each UUID might consume more memory than the objects payload. * Cleanup of the request map in the S7Protocol (and others): I absolutely agree on this. This was one of the things I knew we will be needing but was hesitant to start working on this, as I didn’t have an idea on how to do this Netty-style. Fortunately when looking into the Enthernet/IP drivers internals, the author there is doing exactly this sort of thing: https://github.com/digitalpetri/ethernet-ip/blob/master/cip-client/src/main/java/com/digitalpetri/enip/cip/CipClient.java (Have a look at the “pending” and “timeouts” properties). * Making the items immutable … I agree :-) But as I mentioned, this is just my opinion. Chris
