current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

After playing around with Java 8 and Generics for some time, I came across an idea while reading some posts about Apple's new programming language Swift.

Swift has AOP-like methods for properties (willSet and didSet) and I thought of creating something similar in Java with the help of the new functional interfaces of Java 8.

/**
 * Generic wrapper for a property with the possibility to apply functions before, after and instead of getting/setting the value
 *
 * @author Thomas Eizinger
 * @param <T> The type of the wrapped value
 */
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.Function;

public class Property<T> {

    private T value;

    private BiConsumer<T, T> beforeSet = (u, v) -> {};
    private Function<T, T> onSet = (v) -> (v);
    private Consumer<T> afterSet = (v) -> {};

    private Consumer<T> beforeGet = (v) -> {};
    private Function<T, T> onGet = (v) -> (v);
    private Consumer<T> afterGet = (v) -> {};

    public void set(T value) {
        this.beforeSet.accept(this.value, value);
        this.value = this.onSet.apply(value);
        this.afterSet.accept(this.value);
    }

    public T get() {
        this.beforeGet.accept(this.value);
        this.value = this.onGet.apply(this.value);
        this.afterGet.accept(this.value);
        return this.value;
    }

    public void beforeSet(BiConsumer<T, T> beforeSet) {
        this.beforeSet = beforeSet;
    }

    public void onSet(Function<T, T> onSet) {
        this.onSet = onSet;
    }

    public void afterSet(Consumer<T> afterSet) {
        this.afterSet = afterSet;
    }

    public void beforeGet(Consumer<T> beforeGet) {
        this.beforeGet = beforeGet;
    }

    public void onGet(Function<T, T> onGet) {
        this.onGet = onGet;
    }

    public void afterGet(Consumer<T> afterGet) {
        this.afterGet = afterGet;
    }
}

Example usage:

public class Foo {

    private Property<Integer> bar;

    public Foo() {
        bar = new Property<>();
        bar.beforeSet((oldValue, newValue) -> System.out.printf("bar will be set from %d to %d.%n", oldValue, newValue));
    }

    public Integer getBar() {
        return bar.get();
    }

    public void setBar(Integer bar) {
        this.bar.set(bar);
    }
}

Some usecases I could think of would be:

  1. Notifying observers
  2. Constraint checks before setting values
  3. Converting values (e.g. tolowercase for strings)

What do you think of this? Does this put too much logic into properties?

Edit:

I changed the following things based on the review and published the code on github:

  • Added null-checks before adding new delegates.
  • New delegates are appended after the existing one.
  • Changed onGet to Consumer<T> and removed beforeGet and afterGet.
  • Changed name to PropertyDecorator
share|improve this question

1 Answer 1

up vote 3 down vote accepted

My first thought is: Interesting.

The Property<T> class as it is right now

First, some feature-requests or changes you can consider:

  • You could combine beforeSet and onSet into a BiFunction<T, T, T> (which is the same as BinaryOperator<T>).

  • It could potentially be useful to use the afterSet as a BiConsumer, to also let it know about what the previous value was.

  • What if a previous function has been set for afterSet/beforeSet/etc.? I think you should do a null-check there and either throw an exception if it has already been set, or auto-add it (using the default methods that exists on the interfaces), such as this.afterSet = this.afterSet.andThen(afterSet);

  • I'm not a big fan of the onGet feature. In my experience, getters should do what they do best: Get the actual value. Having the possibility to modify the value that should be returned seems risky. Doing conversion like String.toLowerCase() for example should lie where it is being used, or the property should be set to lower case right from the start IMO.

  • Actually, thinking about it... I'm not sure about any of the get-features... I just don't see a specific real use case for them.

The system itself

I'm going to list a couple of good things and a couple of bad things with this approach:

Good things:

  • Allows you to manipulate the "behavior" of a property in quite some detail.
  • Allows you to manipulate the "behavior" of a property in quite some detail.
  • Allows you to manipulate the "behavior" of a property in quite some detail.

(One time for the beforeSet feature, once for the onSet and once for afterSet. Three-in-one)

Bad things:

  • Primitive types needs to be wrapped into it's non-primitive type, like int -> Integer. This leads to slower code as there will be some auto-boxing, and it will allow the value null where you might not want that value, or you have to handle the null with a beforeSet or similar. Although you could potentially separate classes for IntProperty, BooleanProperty, etc. I don't think you'd want to do all those classes.

  • When should I make my Java classes use this approach or not? Should I introduce them when I feel like I need them or should I play safe and use this approach on all my properties, even though when I will probably never need them? I can imagine there will be some headache going on no matter what option I would choose.

  • It's breaking the Java beans style, which isn't a bad thing by itself. It can cause a bit of a problem when using libraries that depends a lot on reflection, but there are normally ways to work around that. (I.e. telling the library to ignore the private property and use the getters and setters provided).

A different approach that's capable of doing the same thing

Consider Google's EventBus code. With the right event classes, this can accomplish the same things. Example code following: (not tested, but something similar would work)

public class Foo {

    private int bar;
    private EventBus events = new EventBus();

    public Foo() {
        bar = 42;
        events.registerListener(this);
    }

    @Subscribe
    private void onSetChanged(BarChangeEvent event) {
        System.out.printf("bar will be set from %d to %d.%n", event.getOldValue(), event.getNewValue()));
    }

    public int getBar() {
        return bar.get();
    }

    public void setBar(int bar) {
        events.post(new BarChangeEvent(this.bar, bar));
        this.bar = bar;
    }
}

Of course, there's bad things to say about this approach as well. One example is that it always creates a new object whenever bar is set. For most projects of course, the @Subscribe method is probably not inside the Foo class. Instead the class will have to expose the EventBus somehow (or the EventBus can be injected into the Foo class).

Still another approach

This is the fastest approach, but unfortunately it is the least flexible approach as well.

The simple "do-it-in-the-setter approach"

public class Foo {

    private int bar;

    public Foo() {
        bar = 42;
    }

    public int getBar() {
        return bar;
    }

    public void setBar(int bar) {
        System.out.printf("bar will be set from %d to %d.%n", this.bar, bar);
        this.bar = bar;
    }
}

Summary

You might have use for your Property<T> code, if so then great. I personally would probably not use it. (I tend to use the Event-driven approach when I need to, not for each and every property of course. Sometimes I might even use the "put-it-in-the-setter-approach")

Does this put too much logic into properties?

Personally, I think it does yes. But that's just my two three cents.

share|improve this answer
    
I previously had a beforeSet method that used a BiFunction but for the sake of seperation I created 2 methods. The problem with the combined approach is, that you must return a value in the function you are inserting, even if you just want to do some constraint checking. Reassigning a method & null-check: I initialized all functions with an empty lambda to avoid the null check. I will consider the andThen approach though, thanks for the hint. Get-Features: Modifying the value here can be risky yes, but isn't this the case with every AOP-like approach? –  Thomas Eizinger Jun 9 '14 at 22:34
    
Null values: I think the user is responsible to handle cases where HIS value is null. The function itself can never be null, despite the user explicitly assings null as the function. But I would much rather catch that in the setter and reassign an empty lambda instead of adding null checks to the actual getter/setter of the property. Autoboxing: Is this really a performance issue here? EventBus: Thanks for the link, but that is just one example I thought of using my class: Raise the event in one of the delegates which results in "nicer" code IMO. –  Thomas Eizinger Jun 9 '14 at 22:40
    
@ThomasEizinger Get-Features: Honestly I've never done any AOP, so perhaps someone else can comment about that aspect (ha-ha). Constraint-checking: If you do constraint-checking in the BiConsumer than what checks can you do really? You can throw an Exception, but that's it. I would have expected the BiFunction to deal with the constraint checking. Reassigning method: Yes, I forgot about that. You're right, checking for null would be pointless there. You're default initializations are good. Null values: Yes, you're right, the user is responsible. –  Simon Forsberg♦ Jun 9 '14 at 23:36
1  
Constraint-checking: I cannot think of a constraint that it is not worth throwing an exception if it does not pass its check. My thoughts on the seperation were: beforeSet: The method is provided both values so you can compare old vs new but modification will not have any effect. onSet: The method is provided only the new value which if modified will be persisted. afterSet: The method is provided the value that has been set and again modification will not have any effect on the stored value. I actually like the seperation into 2 read-only delegates and one with write access. –  Thomas Eizinger Jun 10 '14 at 0:29
2  
I tend to disagree with the performance issue raised on autoboxing. This code will (should) never be used for performance-critical situations either way, so I think you can get away with the boxing. –  skiwi Jun 10 '14 at 9:44

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.