Hi Andrey, are you proposing a Factory-Factory? Wouldn't that go a little too far? Currently the request factory method being in the connection, is just an implementation detail we could have a S7Reader class implementing the Reader interface and this is generated from the connection, if this is the problem.
Chris Am 12.09.18, 15:54 schrieb "Andrey Skorikov" <[email protected]>: Hi Sebastian, good point! The mutability of PlcReadRequest would be a consequence of the mutability of the PlcConnection (or something that can handle the execute). However, in order to construct a immutable PlcReadRequest, currently we still need to obtain a PlcRequest.Builder from the same mutable PlcConnection. I think, if we really want PlcReadRequest to be immutable, then we should be able to construct them independently (not from a mutalbe object). Perhaps we could separate PlcRequest construction from its exection by providing a RequestBuilder factory method not on a mutable PlcConnection but "higher up", for example somewhere on a PlcDriver? Regards, Andrey On 09/12/2018 03:33 PM, Sebastian Rühl wrote: > Hey Andrey, > > One thing that would stand against this: Adding a execute() would make the PlcReadRequest mutable which is a thing we should avoid (Mutable because it would need a reference store to something that can handle the execute). > > FYI: I added a mutability test into plc4j-api (which should be added to plc4j-driver-bases after the refactoring) which tests all Items for mutability. Currently we have some open issues regarding that. > > Sebastian > >> Am 12.09.2018 um 14:50 schrieb Andrey Skorikov <[email protected]>: >> >> Hello, >> >> currently, PlcReadRequests and PlcWriteRequests are executed by invoking the corresponding read/write operation on the PlcReader/PlcWriter objects. Hence the typical workflow for reading a value contains the following steps: >> >> 1. Obtain a PlcReader instance from a PlcConnection >> 2. Obtain a Builder by invoking readRequestBuilder() on PlcReader >> 3. Build a request using the Builder >> 4. Execute the request by invoking read() on the PlcReader, passing the constructed request as argument >> >> PlcReader reader = ... // obtain reader >> PlcReadRequest request = reader.readRequestBuilder().addItem(...).build(); >> Future<PlcReadResponse> response = reader.read(request); >> >> Since we only can build a request throgh a PlcReader instance, invoking the read operation on the PlcReader is redundant. Therefore I suggest removing the read/write operation from the PlcReader/PlcWriter and defining a execute() operation on PlcRequest instead. The workflow would look like this then: >> >> PlcReader reader = ... // obtain reader >> PlcReadRequest request = reader.readRequestBuilder().addItem(...).build(); >> Future<PlcReadResponse> response = request.execute(); >> >> Please note that there is no more need to "keep" a reference to PlcReader in this context, since it is implicitly associated with the request by calling reader.readRequestBuilder().build(). >> >> Best regards, >> Andrey >>
