Hi, despite the unchecked requests, having typed request items is a great improvement. And as the current changes are not breaking anything it would be good to have these changes in master (so they can be applied to the Scala API without having double work)
Also Chris’ idea using getValue is quite cool, let’s try to apply it :) From my perspective having the changes in master is already an improvement. Mark > Am 04.01.2018 um 18:47 schrieb Christofer Dutz <[email protected]>: > > Hi Sebastian, > > What I did think about, would be to have a getValue method in the response > object, which you can pass in a request item reference. > The method should return the value for that particular request item in the > generic type the requestItem defined. > > I think the unchecked version of the Read/WriteRequest should not have the > convenience constructors the generic version has. So, it should only have a > no-args and a var-args Constructor that only accepts request items and > eventually a list of request Items. > > And how about making the generic version extend the non-generic version? > After all generics are just syntactic sugar that’s used by the compiler. So > instead of setting this to the most generic type of object, I would rather > opt to have the generic version extend the non-generic one with generic type > and methods. > > But I’m open for other suggestions and/or opinions. So what do you all think? > > Chris > > > > Am 04.01.18, 17:36 schrieb "Sebastian Rühl" > <[email protected]>: > > 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 >
