viritin / viritin

The "commons" library for Vaadin developers
https://vaadin.com/addon/viritin
Apache License 2.0
153 stars 77 forks source link

Add interfaces with fluent setters #145

Open maxschuster opened 8 years ago

maxschuster commented 8 years ago

I really like to use viritins fluent setters. They are a good way to reduce boilerplate code and also make the code more readable.

But I often find that a fluent setter is missing on the component that I am using and I have to use the ugly default setter ... :wink:

I would suggest to add interfaces to the add-on to make sure every component has all common fluent setters. This will also make documentation a lot easier because components can inherit it from the interface. And it with an IDE it would not be a problem to override the interface methods with the right return type.

This is how I think it could look like:

public interface FluentComponent extends Component {

    /**
     * Doc...
     */
    public FluentComponent withWidth(String width);

    // more methods ...

}

public interface FluentField<T> extends FluentComponent, Field<T> {

    /**
     * Doc...
     */
    public FluentField<T> withValue(T value);

    // more methods ...

}

// maybe more interfaces ...

What do you think? Will it work and benefit the usability?

I can try to implement it if you think its a good idea.

dve commented 8 years ago

If moving to java 8 is an option, I would suggest the following:

public interface FluentApiComponent<C extends Component> {
    public default C witWidth(String width) {
        ((C) this).setWidth(width);
        return (C) this;
    }
}
public interface FluentApiField<F extends Field<T>, T> extends FluentApiComponent<F>{
    public default F withValue(T newValue){
        ((F)this).setValue(newValue);
        return((T)this);
    }
}

This would prevent duplicating code in every component

mstahv commented 8 years ago

Both suggestions are excellent. Java 8 is gaining its usage fast, but I'd still like to support Java7 for a while, rough guess, for about a year or at least until autumn. Should see to result form our last community survey for the final decision.

If @maxschuster you want to start the project with "oldschool" way, feel free, I'll pull in all the changes. Otherwise I'll (personally) wait with this until we drop Java 7 support.

maxschuster commented 8 years ago

A class can not implement the same interface with different generics as far as I know. So we can not use it for abstract classes like AbstractElementCollection etc.

maxschuster commented 8 years ago

I have started to create the necessary interfaces inside the fluency branch.

I have also included generics as suggested by @dve , so it can be upgraded to java 8 later. If we have a deeper class hierarchy like with MButton, we still have to override the methods manually.

This is an example how it looks like:

public interface FluentAbstractField<C extends AbstractField<T> & FluentAbstractField<C, T>, T> 
        extends FluentAbstractComponent<C>, FluentField<C, T>, 
        FluentProperty.FluentReadOnlyStatusChangeNotifier<C> {

    public C withConverter(Class<?> datamodelType);

}
mstahv commented 8 years ago

@maxschuster, how confident our you with the fluency branch? Is there still something relevant missing? Should we start thinking of merging the stuff to master?

maxschuster commented 8 years ago

Hi @mstahv, sorry for the late reply.

I am still working on this branch. But the code for implementing the the interfaces really polutes the Component's class. I think @dve s autogenerated classes from #177 would be a great way to deal with that problem.

Another problem I came across are inconsistent setter method names. Most of the time fluent setter methods start with "with" but some start with "set" if the name was not already taken by the original Vaadin component. But maybe I sould create seperate issue for that.

maxschuster commented 8 years ago

I have got some doubts about copy/pasting javadoc from the Vaadin Core to the fluent setters, I hope I am not violating the copyrights of vaadin.

I have created a forum topic for this question: https://vaadin.com/forum#!/thread/12892178

mstahv commented 8 years ago

Just do it, let's blame me if somebody wants someone to blame :-) If you feel really guilty, you can add comments like "// Javadoc copied form Vaadin Framework" above each comment snippet you "steal".