Ah and I missed the suggestion to rename the VarParameter to something with
Command in it:
This is a well-established name for this part of the package, so you will find
reference to parameters all over the internet. Secondly these classes are all
internal classes used by the driver implementation. No user should ever use
these, so I think we should stick to the well-established names.
Chris
Am 02.08.18, 10:47 schrieb "Christofer Dutz" <christofer.d...@c-ware.de>:
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
