Functional style

[nayrban]

Hi @gofabian I do have some comments/suggestions if you want to discuss, is all related to verbosity and functional style

https://github.com/gofabian/spring-boot-data-r2dbc-jooq/blob/45c0162035e641b4b07fe413ff87d1148819a15d/src/main/java/gofabian/r2dbc/jooq/ReactiveRecordExecutor.java#L179

You can turn that into this, since this is all a lazy evaluation when the first match happen the execution is completed, you can also another indirection layer with Optional.ofNullable(configuration.data()) I know that method is unused so is perfect because we can avoid side effects

return configuration.data().keySet().stream() .filter(keyString::equals) .findFirst();

https://github.com/gofabian/spring-boot-data-r2dbc-jooq/blob/45c0162035e641b4b07fe413ff87d1148819a15d/src/main/java/gofabian/r2dbc/jooq/ReactiveRecordExecutor.java#L43
Or even that into this

boolean executeUpdate = Arrays.stream(record.getTable().getPrimaryKey().getFieldsArray()) .noneMatch(field -> isNewRecord(record, field));

Reference https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/stream/Stream.html#noneMatch(java.util.function.Predicate)

[gofabian]

https://github.com/gofabian/spring-boot-data-r2dbc-jooq/blob/45c0162035e641b4b07fe413ff87d1148819a15d/src/main/java/gofabian/r2dbc/jooq/ReactiveRecordExecutor.java#L179

You can turn that into this, since this is all a lazy evaluation when the first match happen the execution is completed, you can also another indirection layer with Optional.ofNullable(configuration.data()) I know that method is unused so is perfect because we can avoid side effects

return configuration.data().keySet().stream() .filter(keyString::equals) .findFirst();

As you said this part of the code is unused at the moment. I copied parts from JOOQ code and converted it into reactive style. I commented this "batch" functionality out as it was not important in the first step.

I opened an issue to check whether this part is still relevant: https://github.com/gofabian/spring-boot-data-r2dbc-jooq/issues/29

https://github.com/gofabian/spring-boot-data-r2dbc-jooq/blob/45c0162035e641b4b07fe413ff87d1148819a15d/src/main/java/gofabian/r2dbc/jooq/ReactiveRecordExecutor.java#L43

Or even that into this

boolean executeUpdate = Arrays.stream(record.getTable().getPrimaryKey().getFieldsArray()) .noneMatch(field -> isNewRecord(record, field));

Reference https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/stream/Stream.html#noneMatch(java.util.function.Predicate)

Thanks for your suggestions! There is always a way to reduce code verbosity. As code style can be a very personal choice I would like to understand your intentions. Which advantages would we have with this change?

For me readability is more important than verbosity because code is read more often than written. So verbose but readable code is fine for me.

This section is copied from JOOQ. Another advantage to keep it like that is that we can compare it to the original code more easily.

What do you think?

[nayrban]

Yeah you are right, is more a personal choice, and as you know is nothing related to performance or efficiency, I have 2 things in mind with that, 1 related to verbosity, and this is obviously a personal choice, not just in the sense of reduce the code but also on how to understand the steps of an operation, I know sometimes is shocking to read a portion of chaining code but I think the guys behind this API put a lot of effort to have a semantic meaning for each operation, so for me is more readeable, but lets just not to pay much attention on this since is a personal opinion.

2 I was thinking more in functional programming style since you are using a reactive library that is focused on that, but again is just my perspective.

Another point that I have, talking about readability, is about anonymous functions we tend to use something(value -> { //block of code })

in my opinion we can extract that block of code into a new meaningful method that can make the code more readeable, without a good name you have to read the code to understand what the purpose of that block into the chain

In any case my intention was just to.know your thoughts on that.

Thanks for your reply.

[gofabian]

Closing the issue because of inactivity.