Skip to content

Conversation

@mp911de
Copy link
Member

@mp911de mp911de commented Mar 12, 2019

MappingR2dbcConverter now considers custom conversions for inbound and outbound conversion of top-level types (Row to Entity, Entity to OutboundRow) and on property level (e.g. convert an object to String and vice versa).


Related ticket: #59.

@mp911de mp911de requested a review from schauder March 12, 2019 10:30
Copy link
Contributor

@schauder schauder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just two minor comments.

public OutboundRow convert(Person source) {
OutboundRow row = new OutboundRow();
document.put("_d", source.getId());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be row.put ...

* @param value must not be {@literal null}.
* @return the {@link SettableValue} value for {@code value}.
*/
public static SettableValue from(Object value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This creates an asymmetry I don't like too much. With one argument you have a factory method, with two args you have to use the constructor.

I'd vote for either only factory methods or constructors but not mixing them in the public API.

MappingR2dbcConverter now considers custom conversions for inbound and outbound conversion of top-level types (Row to Entity, Entity to OutboundRow) and on property level (e.g. convert an object to String and vice versa).
@mp911de mp911de force-pushed the issue/59-apply-conversions branch from d2e7333 to b68db1b Compare March 18, 2019 07:32
schauder pushed a commit that referenced this pull request Mar 19, 2019
schauder pushed a commit that referenced this pull request Mar 19, 2019
MappingR2dbcConverter now considers custom conversions for inbound and outbound conversion of top-level types (Row to Entity, Entity to OutboundRow) and on property level (e.g. convert an object to String and vice versa). Original pull request: #70.
@mp911de
Copy link
Member Author

mp911de commented Mar 20, 2019

That's merged via 6654db3.

@mp911de mp911de closed this Mar 20, 2019
@mp911de mp911de deleted the issue/59-apply-conversions branch March 20, 2019 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants