roookeee / datus

datus enables you to define a conversion process between two data structures in a fluent functional API
MIT License
41 stars 5 forks source link

Conditionally apply a transformation #36

Closed wind57 closed 3 years ago

wind57 commented 4 years ago

Suppose I have two DTOs:

public class In {

    public String getAge() {
        return age;
    }

    public void setAge(String age) {
        this.age = age;
    }

    private String age;
}

and

public class Out {

    public Integer getAge() {
        return age;
    }

    public void setAge(Integer age) {
        this.age = age;
    }

    private Integer age;
}

And I want to conditionally apply a transformation: I want to map String age only if it is made from digits, otherwise simple don't map it.

The only way I found to do this is:

        In in = new In();
        in.setAge("no");

        Out out = Datus.forTypes(In.class, Out.class)
                       .mutable(Out::new)
                       .from(In::getAge).given(Pattern.compile("\\d+").asPredicate(), Function.identity())
                       .orElseNull().map(x -> x == null ? null : Integer.parseInt(x)).into(Out::setAge)
                       .build()
                       .convert(in);

        Assertions.assertNotNull(out);
        Assertions.assertNull(out.getAge());

which looks weird, at best. Did I miss some documentation and there is a better way? If no, I think I might submit a PR with my idea to handle this; unless you already have something like this in motion.

roookeee commented 4 years ago

As such a feature would be impossible in the immutable API (every constructor parameter has to have a complete mapping definition that always returns a value) there is currently not a feature to just stop a .from(..)-chain to have a homogenous API for both the mutable and immutable API. Thinking about it I have no issues to implement a stop() (or whatever you want to call it) but I then have to copy&paste the ConditionalEnd class which is currently used by both APIs. Will give this some thought later.

wind57 commented 4 years ago

I want to add two things here:

@Mapping(target = "age", ignore = true) // ignored

and you can later do whatever you want with it in :

@BeforeMapping
default void beforeMapping(@MappingTarget Out target, In source) {
             // handle the mapping here
}

You could work-around this the same way: don't provide the mapping in from(...) and instead have additional methods in Datus/Mapper.

roookeee commented 4 years ago

Random thought I will document so I won't forget: basically nullsafe() would make the above example more readable so we could introduce a nullsafe() variant wherein one can provide a Predicate<T> which on-match propagates the original value / some provided value to the destiantion setter / constructor parameter.

Regarding your comment: I for one really dislike unless alike constructs wherein you say do xyz, but not if z as for me its less readable.

roookeee commented 4 years ago

I thought of three possible solutions:

  1. Convert the input to an Optional at the start and do .into(v -> v.orElse(null)) (does not need new datus code)
  2. Implement a guard statement which stops the mapping process and forces the user to handle the "stopping" in the to / into steps (extensive changes are required to make this work)
  3. Misusing the nullsafe() guard with a new detection mechanism if the null short-circuiting worked applied (e.g. onNull(...)), i could even do overloads for to / into to supoort fallbacks for null more easily (the changes that are required to make this work are limited in scope)

The first approach is boilerplatey whilst the second approach requires quite a bit of duplication while also increasing the complexity of the internal code & public API - it's a feature that is not as easy to grasp. The third approach is the most tempting for me as its a compromise between boilerplate vs. complexity.

I can see your use case but as explicitly stated in datus documentation: if you are doing a transformation that needs this kind of conditional processing I would advise you not to use datus mappers. The main idea of datus is to remove "dumb" data transforming factories that are not test-worthy but this kind of transformation that considers Regexes etc. subjectively is worthy of being its own unit and thus should be under test (you can of course still implement the Mapper<A,B> interface to be consistent).

Will give this some more thought. If this issue is a deal breaker / has a deadline for you let me know; I would be willing to pick up the pace then :)

roookeee commented 3 years ago

Some time has passed and I tried tinkering with the above mentioned ideas. Sadly I found no practical implementation as I would need some sort of generic generics (<U, T<U>>) to make it work in a sensible fashion. Even then I would have to break backwards compatability.

In regards to your implementation: I still advice using .nullsafe() to remove some boilerplate, but there is an even more elegant solution to this:

        Mapper<In, Out> mapper = Datus.forTypes(In.class, Out.class)
                .mutable(Out::new)
                .from(In::getAge).given(Pattern.compile("\\d+").asPredicate(), age -> Integer.parseInt(age)).orElseNull()
                .into(Out::setAge)
                .build();

Regards, roookeee

wind57 commented 3 years ago

this is indeed nice and I will take care to transform it in our code base like this. yes, we use your project in production, I was really bored of map-struct and my personal effort on such a project ran out of time. thank you for it.