davidmarquis / fluent-interface-proxy

Forget boiler plate code for your Java Fluent builders! This project provides a proxy that implements your Builder interfaces dynamically (no code required!)
MIT License
55 stars 17 forks source link

Could you please help to consider to support the super interface for the builder? #3

Closed charleech closed 8 years ago

charleech commented 10 years ago

Dear Sir,

Firstly I would like to thank for providing the great and brilliant tools. I'm very appreciated.

In my environment, my bean is not a stand alone and it is derived from another super class. In fact it is a JPA data bean. Here is some example.

public class MySuperBean() {
    private String id;
    private String description;
    private Calendar createdDate();
    private String createdBy();
    //...Other sharing common attributes.

    //Getters/Setters
}

public class MyPerson extends MySuperBean {
    private String firstName;
    private String lastName;
    //...Other specific attributes for Person

    //Getters/Setters
}

public class MyCompany extends MySuperBean {
    private String name;
    private String type;
    //...Other specific attributes for Company

    //Getters/Setters
}

Regarding to the current implementation, I need to create 2 Builder Interface which contains some redundant methods from the super class as the following example

public interface MyPersonBuilder extends Builder<MyPerson> {
    MyPersonBuilder withId(final String id);
    MyPersonBuilder withDescription(final String description);
    // other withXXX from the MySuperBean

    // other withYYY from the specific MyPerson
}

public interface MyCompanyBuilder extends Builder<MyCompany> {
    MyPersonBuilder withId(final String id);
    MyPersonBuilder withDescription(final String description);
    // other withXXX from the MySuperBean

    // other withYYY from the specific MyCompany
}

It would be nice if you may consider to provide the support for the Super Interface Builder as the following example

public inferface MySuperBeanBuilder<T extends MySuperBeanBuilder<T>> {
    T withId(final String id);
    T withDescription(final String description);
     //  other withXXX from the MySuperBean
}

public interface MyPersonBuilder extends MySuperBeanBuilder<MyPersonBuilder>,
                                         Builder<MyPerson> {
    MyPersonBuilder  withFirstName(final String firstName);
    MyPersonBuilder  withLastName(final String lastName);
    // other withYYY from the specific MyPerson
}

public interface MyCompanyBuilder extends MySuperBeanBuilder<MyCompanyBuilder>,
                                         Builder<MyCompany> {
    MyCompanyBuilder withName(final String name);
    MyCompanyBuilder withType(final String type);
    // other withYYY from the specific MyCompany
}

The the actual using may look like the following

    MyPerson person = ReflectionBuilder.implementationFor(MyPersonBuilder.class).
                                        withId("some-id").
                                        withDescription("some-description").
                                        withCreatedBy("my-admin").
                                        ...
                                        withFirstName("Charlee").
                                        withLastName("Ch.").
                                        ....
                                        build();

By overview I also have a chance to dig in your code and found some rough tweak as the following: -

public class BuilderProxy implements InvocationHandler {
      ...
    private boolean isFluentSetter(final Method method) {
        return method.getParameterTypes().length == 1
                && method.getReturnType().isAssignableFrom(this.proxied)
                && !this.isBuildMethod(method);
    }
}

I'm also not sure if it is an acceptable or if it may be any effects to the module or not. Could you please help to consider? Thank you very much for your help in advance. I'm looking forward to hearing from you soon.

Regards,

Charlee Ch.

davidmarquis commented 10 years ago

Hey Charlee, thanks for the suggestion. In the past for similar situations I resorted to duplicating setters in both builders... not ideal (especially if your class hierarchy is large) but it works.

Precautions need to be taken in order to avoid possible misuse of generics on the parent class and through the child class as well. I'll reflect on this and see what can be done...

In the meantime, if your need is urgent you can always try and submit a pull request :) The project can be run quickly using Maven and full test coverage is available so you can easily see if you break anything with a change. I would expect this new use-case to also be covered by a new passing test.

Cheers!

charleech commented 10 years ago

Dear Sir,

I've checked out and started making change. Anyhow I've found some trouble and would like your help to advise as the following: -

1 When I applied the following: -

public class BuilderProxy implements InvocationHandler {
      ...
    private boolean isFluentSetter(final Method method) {
        return method.getParameterTypes().length == 1
                && method.getReturnType().isAssignableFrom(this.proxied)
                && !this.isBuildMethod(method);
    }
}

I've found the exception as the following: -

java.lang.IllegalStateException: Could not imply class being built by builder [
interface com.fluentinterface.domain.dao.CustomerBuilder]. 
If the interface does not extend [interface com.fluentinterface.builder.Builder],
you must explicitly set the type of object being built using the 'builds(class)' method.

During creating the builder, I need to mentions the target class explicitly as the following: -

return implementationFor(CustomerBuilder.class)
                .builds(MyPerson.class)
                .create();

2 When testing against the FieldAttributeAccessStrategy. I've found another exception as it cannot get the field from super class as the following: -

java.lang.IllegalStateException: Method [withId] on 
[interface XXX] corresponds to unknown property [id] 
on built class [YYY]

I need to tweak the FieldAttributeAccessStrategy by iterating over its class hierarchy as the following: -

public class FieldAttributeAccessStrategy implements AttributeAccessStrategy {
    ....
    private Field getFieldFromClass(Class<?> clazz, String fieldName) {
        try {
            return clazz.getDeclaredField(fieldName);
        } catch (NoSuchFieldException e) {
            return getFieldsFromHierarchy(clazz, fieldName);
        }
    }

    private static Field getFieldsFromHierarchy(Class<?> clazz, String fieldName) {
        Field result;
        Class<?> parent = clazz.getSuperclass();

        if (parent == null) {
            return null;
        }

        try {
            result = parent.getDeclaredField(fieldName); 
        } catch (NoSuchFieldException e) {
            result = getFieldsFromHierarchy(parent, fieldName);
        }

        return result;
    }
}

Regarding the above, I'm not sure if this change is worth enough or not. Especially the iterating recursively through the class hierarchy. Could you please help to advise further? Thank you very much for your help in advance. I'm looking forward to hearing from you soon.

Regards,

Charlee Ch.

davidmarquis commented 10 years ago

Have a look at my latest commit: 38f6e8a43cf59a32aeefaa928da4fc137629a523

This should fix your issue with having multiple parent interfaces on your main builder and remove the need to specify builds(MyPerson.class) when creating the proxy.

charleech commented 10 years ago

Hi,

I've change the code which follows your review as https://github.com/charleech/fluent-interface-proxy/commit/dc589897113ca4d927a1c32bdde026987e3ee4a5

I'm new to GitHub and do not know how to fetch your change https://github.com/davidmarquis/fluent-interface-proxy/commit/38f6e8a43cf59a32aeefaa928da4fc137629a523 to my forked branch. I will try my best on tomorrow morning. (I'm at GMT+7 time zone, right now is 8.00PM. :) )

Could you please help to advise further? Thank you very much for your help in advance. I'm looking forward to hearing from you soon.

Regards,

Charlee Ch.

davidmarquis commented 10 years ago

To get the latest changes from my master branch, just run: git pull https://github.com/davidmarquis/fluent-interface-proxy.git master in your local working directory

charleech commented 10 years ago

Hi,

I've tested against the https://github.com/davidmarquis/fluent-interface-proxy/commit/38f6e8a43cf59a32aeefaa928da4fc137629a523. It works perfectly.

I also have changed the test case to remove the mentioning the target build class explicitly as https://github.com/charleech/fluent-interface-proxy/commit/e1d45d8c81ee0838c05dc5a45f2be25abe38b594

Could you please help to review and advise further? Thank you very much for your help in advance. I'm looking forward to hearing from you soon.

Regard,

Charlee Ch.

aludwiko commented 9 years ago

charleech - if you need inheritance support, you can check this project: https://github.com/aludwiko/fluentbuilder davidmarquis - I wish I had found your project before I start my own. It would be easier to implement few things. Anyway, you can verify how dynamic proxy can be combined with automatic code generation.