andvasp commented on code in PR #2674:
URL: https://github.com/apache/plc4x/pull/2674#discussion_r3750085705
##########
plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/base/EipTcpConnection.java:
##########
@@ -278,11 +278,21 @@ private CompletableFuture<Void> probeAttributes() {
});
}
- private CompletableFuture<Void> checkAttributesSingle() {
+ private CipService getCipService(EipPacket response) {
Review Comment:
> A simple getCipService would make me expect that it simply gets the CIP
service ... here the method is actually extracting something if a very specific
data-case is present or doesn't do anything if that's not the case.
Yes. extractCipService is a better name. Actually, I thought about using it
when thinking about possible solutions but when implementing it I forget to use
it.
> Are we using this or could we use this in different places?
I am using this method at 2 places. I found other places that could use it
but as they do not have exactly the same logic, I prefer to not change them.
But I believe would be good to use. Let me know what you think.
> In the past we used a lot of "return null" methods and are more and more
trying to use Optionals in Java ... I tink renaming it to something that
indicates the fact that it's not just blindly accessing something, possibly
something like:
>
> ```
> Optional<CipService> extractCipService(EipPacket response)
> ```
I considered using Optional, but in this case I found it more verbose and as
this method is used just internally in this class and with Pattern Matching, I
prefer to not use Optional here. See the examples below and tell me what do you
think.
```java
if (service.isPresent() && service.get() instanceof
GetAttributeSingleResponse gsr) // I do not like to use isPresent() and get()
together but I believe is less verbose.
```
I considered create the method below but again I think is more verbose and
it with extra comparations at Class methods.
```java
<T> Optional<T> extractCipService(EipPacket response, Class<T> type)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
