Omit writeToParcel and CREATOR when already defined.

[grandstaish]

No-op if everything is already defined.

[grandstaish]

I'm not 100% if I should be checking for FINAL here. I'm also not 100% on whether or not I should be checking the type argument of the creator either. Both of those things aren't mandatory. Thoughts?

[grandstaish]

This is the implementation of what you described in #26

[rharter]

This looks like a good start. I'll have to think on the final, I guess I hadn't considered that not being mandatory.

Could you please add test cases for this?

[grandstaish]

Yep. I'll get onto the test cases tomorrow

Edit: I lied. I got bored and did them today.

[grandstaish]

Not sure if you want this class moved elsewhere. This class has to have a static member which isn't allowed using the existing standard of having inner classes within AutoValueParcelExtensionTest.

[rharter]

This should be able to be done via the standard testing methods, which aren't inner classes, but JavaObjects whose content is provided via String.

[grandstaish]

This is copying the format of your 'throwsForNonParcelableProperty' and 'acceptsParcelableProperties' tests. Happy to change if you have a suggestion as for how. I'm not sure how to check the lack of parcelable generation though. I don't want to test the output of AutoValue itself, because that's all implementation details that you don't control.

[JakeWharton]

I'm personally against allowing this at all. What is the use case being addressed here?

But, if support for it is going to be allowed then two things strike me as being needed:

• Emit an error if writeToParcel is present without CREATOR (or vise versa). Allowing this would be violently error prone.
• Emit a warning on the type when both writeToParcel and CREATOR are matched and the implementation is not generated. Even when both of these are present it is still a highly error prone piece of code that should be avoided when this extension is present.

[grandstaish]

Good points. I'll leave it to you guys to decide if implementing writeToParcel and the CREATOR yourself should be allowed, or whether the behaviour in this PR is the desired one. I have no strong opinion on this, especially because I can't see why anyone would try to write these parts themselves. I wrote this based on the comment in #26 to make it consistent with how AutoValue usually works.

If you want to not allow implementing these things, I'm happy to close this PR. If you want this behaviour, I'll implement the error checking that you mentioned.

[rharter]

Those are good points. I don't have strong feelings either way, was just thinking of how AutoValue usually works. It does make sense, though, that you should write both, or neither, but one or the other, as that would be super error prone.

[grandstaish]

Do you want me to implement Jake's suggestion @rharter? I can make it fail on compilation and update the tests.

[rharter]

That sounds good. Thanks!

On Mon, Apr 11, 2016, 1:34 AM Bradley Campbell [email protected]
wrote:

Do you want me to implement Jake's suggestion @rharter
https://github.com/rharter? I can make it fail on compilation and
update the tests.

—
You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#46 (comment)

[grandstaish]

I'll re-open a new PR for the change.

[m-sasha]

It's unfortunate that this was your decision. There are valid cases where you want to allow custom parcelling implementations. Why would you want to allow custom Parcelable implementations for fields, but not for root types?

Specifically, I have two classes with circular dependencies between them: Parent, that has a list of Children, and each Child, in turn, holds a reference to its Parent. Without a custom Parcelable implementation, I can't use AutoValue on this class.

public class Parent implements Parcelable{

    private final List<Child> children;

    private Parent(List<Child> children){
        this.children = children;
    }

    public static Parent newParent(List<Child> children){
        Parent parent = new Parent(children);
        for (Child child : children)
            child.setParent(parent);
        return parent;
    }

    private Parent readFrom(InputStream in){
        List<Child> children = readChildrenFrom(in);
        return newParent(children);
    }

    public static class Child{

        private final Parent parent;

        private Child(){
        }

        private void setParent(Parent parent){
             this.parent = parent;
        }
    
        private Child readFrom(InputStream in){
            // Read Child from InputStream
        }

    }

}

With the above, I can implement Parcelable for Parent by calling newParent instead of the constructor. I can't do it with auto-value-parcel.