Hi Chris,

Thanks for the feedback.

> One thing I did notice, was that one ReadRequest can have different types of 
> items inside … should the ReadRequest / WriteRequest be fixed to one generic 
> type in both cases?
> 
> Something like this:
> 
> PlcReadRequest req = new PlcReadRequest();
> req.addItem(new ReadRequestItem(Byte.class, inputs));
> req.addItem(new ReadRequestItem(Boolean.class, outputs, 3));
> CompletableFuture future = plcReader.read(req);
> PlcReadResponse resp = (PlcReadResponse) future.get();

I pushed a change so you can do this:
UncheckedPlcReadRequest req = new UncheckedPlcReadRequest();
req.addItem(new ReadRequestItem<>(Byte.class, inputs));
req.addItem(new ReadRequestItem<>(Boolean.class, outputs, 3));
CompletableFuture future = plcReader.read(req);
PlcReadResponse resp = (PlcReadResponse) future.get();
Without getting warnings.
> 
> It does produce warnings, the way it’s currently implemented.
> Right now, my version allows using all types of items inside one request 
> (even mixed), but we lost generification. Eventually it would be good to have 
> a Read/WriteRequest and a Batch version that’s not generic.

I tried now a couple of variants with the generics and I have found no good way 
to represent the batch version beside using the „unchecked“ requests.

> 
> What do you think? Right now, I would opt to not merge the changes back until 
> we have discussed this fully.

In my opinion in really depends on the usage later on. If you mainly using a 
single request then the generics might help. If your most use case is the 
UncheckedPlcReadRequest later anyway then this generic might not help at all. 
Maybe we can find a way to let the user define own messages and keep type 
safety.
What we could do is to split the Plc(Read|Write)Request into two classes with 
one single and one bulk and move the common code into an abstract plc request.

> 
> 
> Chris

Sebastian

Reply via email to