Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

partial private default constructor for Java framework interop (JPA...) #1405

Closed
sgalles opened this issue Aug 18, 2015 · 39 comments
Closed
Assignees
Milestone

Comments

@sgalles
Copy link

sgalles commented Aug 18, 2015

If we could write this :

class Foo{
    abstract new(){}
}

it would allow to generate a private default constructor in Java for the frameworks that need it.

For instance, currently this JPA entity with FIELD access
https://github.com/sgalles/ceylon-dddsample/blob/master/source/dddsample/cargotracker/domain/model/cargo/Cargo.ceylon
must have this constructor to please the compiler.

shared new() extends init(TrackingId(), RouteSpecification()){}

but it could simply be
abstract new(){}

@sgalles
Copy link
Author

sgalles commented Aug 18, 2015

FTR : ceylon/ceylon-compiler#2250 removed a part of the pain with late but I'm inclined to think that AccessType.PROPERTY is not super useful in Ceylon because the visibility of the generated getter/setters is always symetrical (I can't have a public getter and a private setter on the same field)

@gavinking gavinking added this to the 1.2 milestone Aug 29, 2015
@gavinking gavinking self-assigned this Aug 29, 2015
@sgalles
Copy link
Author

sgalles commented Aug 29, 2015

uhm...on second thought, something still bothers me, but I don't know how this can be solved.
OK, let's say that I'm now able to declare a partial private default constructor.
Now all my JPA entities looks like this :

class Foo{
    abstract new(){}
    shared new init(String bar, String baz){} // yuck !
}

in a perfect world, I'd like to be able to use the classes of my entities as perfectly normal classes, but the partial constructor trick leaks as this unnecessary init constructor.
Actually, I'd like to write

class Foo(String bar, String baz){
    abstract new(){}
}

but this obviously breaks the golden rule of constructors VS class parameters....
...unless this was allowed provided that the private constructor is not called in the class (but I don't know if it would be sound)

@lucaswerkmeister
Copy link
Member

AFAICT this could only be allowed on classes with a nonempty parameter list, otherwise both constructors would have the same Java signature.

@sgalles
Copy link
Author

sgalles commented Aug 29, 2015

yes indeed, @lucaswerkmeister but anyway if I have a class with a an empty parameter list JPA is perfectly happy with the generated public constructor and I don't need the abstract one.

@jvasileff
Copy link
Member

What about an annotation in ceylon.interop.java that tells the compiler to generate a no-arg constructor?

@gavinking
Copy link
Member

So the big question is: do we ever need this default constructor to actually do anything? If it's always empty, then I suppose an annotation in java.lang would be the cleanest solution. Or perhaps we could even make it a side-effect of the new serializable annotation.

But if there are some cases where we do need to do something in this constructor then that solution isn't quite good enough. Thoughts?

@gavinking
Copy link
Member

What about an annotation in ceylon.interop.java

No, not there: the compiler doesn't know about ceylon.interop.java. We would put it in java.lang, I suppose.

@tombentley
Copy link
Member

Or perhaps we could even make it a side-effect of the new serializable annotation.

I'm not sure I understand that, assuming you mean the Ceylon serializable annotation. To me it feels more like this is a requirement stemming from the class needing to be a JavaBean. JavaBeans are also supposed to be java.io.Serializable, of course, so it feels more natural to make this magic happen when a class satisfies that interface.

@gavinking
Copy link
Member

To me it feels more like this is a requirement stemming from the class needing to be a JavaBean. JavaBeans are also supposed to be java.io.Serializable, of course, so it feels more natural to make this magic happen when a class satisfies that interface.

Well, no, not really.

  1. JPA/Hibernate entities are not required to be serializable, they have their whole own serialization mechanism, and
  2. anyway, java.io.Serializable does not require a default constructor, since serialization bypasses constructors.

@gavinking
Copy link
Member

Well, to solve the particular case of JPA, we could just always generate a private default constructor.

@tombentley
Copy link
Member

always generate a private default constructor.

Well the only problem with that is bloat.

@gavinking
Copy link
Member

Well how much bloat is one empty constructor with no parameters?

@tombentley
Copy link
Member

In every class in a module though.

Also doesn't it need to be protected to cater for subclasses?

@gavinking
Copy link
Member

In every class in a module though.

Well we need to measure what is the impact of that in terms of a %.

Also doesn't it need to be protected to cater for subclasses?

Probably.

@tombentley
Copy link
Member

OK, let me try

@gavinking
Copy link
Member

Cool, thanks.

On Thu, Oct 1, 2015 at 11:52 AM, Tom Bentley notifications@github.com
wrote:

OK, let me try


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

Gavin King
gavin@ceylon-lang.org
http://profiles.google.com/gavin.king
http://ceylon-lang.org
http://hibernate.org
http://seamframework.org

@tombentley
Copy link
Member

Problem: What about a Ceylon class with a Java superclass without a nullary constructor? We can't generate a nullary constructor in that case.

@tombentley
Copy link
Member

terms of a %.

0% for the SDK, to 2 d.p. at least.

@gavinking
Copy link
Member

Sure, there are some cases where you shouldn't generate this:

  • classes with an of clause
  • classes where the superclass has no default constructor

Perhaps there are some others, I'm not sure.

@gavinking
Copy link
Member

@tombentley aah, but what happens with final fields? you suppressed that check? Does the VM accept that?

@gavinking
Copy link
Member

Or you assign Java default values?

@tombentley
Copy link
Member

I'm assigning default values. The suppression of javac errors is something which we're only doing in very limited circumstances.

@gavinking
Copy link
Member

Cool, well that's fine.

@tombentley
Copy link
Member

My concern about the corner cases where we don't generate such a constructor is that it might be non-obvious to the user why it suddenly doesn't work, and since it's an implicit thing there's nowhere that we can put an error to say "this isn't going to work". It just mysteriously won't work at runtime.

@tombentley
Copy link
Member

Oh, what about generic classes, btw: We need reified type parameters, but JPA wan't supply them. So I guess we shouldn't generate ctors in that case either.

@gavinking gavinking removed their assignment Oct 1, 2015
@gavinking
Copy link
Member

@tombentley I don't see how they're corner cases. They're cases where it would be wrong/impossible to have a default constructor.

@gavinking
Copy link
Member

Oh, what about generic classes, btw: We need reified type parameters, but JPA wan't supply them. So I guess we shouldn't generate ctors in that case either.

Yes, that seems correct.

@gavinking
Copy link
Member

Related ceylon/ceylon-compiler/issues/2329.

@tombentley
Copy link
Member

This is turning out to be quite difficult because I need the constructor to initialize final reference fields to null. To do that I need to know what those fields are, and it turns out the backend cannot readily answer the question: "what fields will this class have?". Reference attributes are not the problem: It's things like methods specified via a callable where what we generate can depend on details which are not in the model, only in the tree. (In particular there's one case where we do or do not generate a field based on that the specifier expression is).

@gavinking
Copy link
Member

And the javac AST you're building doesn't let you iterate over all the fields you've added to it?!

@tombentley
Copy link
Member

I think we should put this has-a-field information in the model, is that OK with you @gavinking? In fact if we added an int or long bit set to the model, it would have us having to add booleans and getters and setters for all these JVM specific things.

@gavinking
Copy link
Member

I would have thought this information was there in the javac AST.

@tombentley
Copy link
Member

That would work, @gavinking, but the transformers have always steered clear from inspecting the emitted JCTree. It would be too easy to get into ordering problems where we decide X, then something goes and adds Y which invalidates the logic for X. But maybe that's the simplest solution in this case.

@gavinking
Copy link
Member

I mean, while I agree that in general it might be a bad idea, it seems like it's the most correct thing here.

@tombentley
Copy link
Member

This constructor ends up looking an awful lot like the constructor we generate for serializable classes, only nullary, rather than with an ignored parameter of undenoteable type. I suppose we could have the serializable one delegate to the other one, or even not generate it at all, and figure it out at runtime.

@gavinking
Copy link
Member

I mean we could say that only serializable Ceylon classes can be serialized via Java serialization, if there were some advantage to that. (I don't especially see any.)

@tombentley
Copy link
Member

No, I don't really see any either. It just seems like there's a small opportunity here to keep things simpler...

tombentley added a commit to ceylon/ceylon.language that referenced this issue Oct 6, 2015
Also apply it to AssertionError.

Part of ceylon/ceylon-spec#1405
tombentley added a commit to ceylon/ceylon.language that referenced this issue Oct 6, 2015
tombentley added a commit to ceylon/ceylon-compiler that referenced this issue Oct 6, 2015
tombentley added a commit to ceylon/ceylon-compiler that referenced this issue Oct 6, 2015
tombentley added a commit to ceylon/ceylon-compiler that referenced this issue Oct 6, 2015
tombentley added a commit to ceylon/ceylon-compiler that referenced this issue Oct 6, 2015
tombentley added a commit to ceylon/ceylon-compiler that referenced this issue Oct 6, 2015
tombentley added a commit to ceylon/ceylon-compiler that referenced this issue Oct 6, 2015
tombentley added a commit to ceylon/ceylon-compiler that referenced this issue Oct 6, 2015
@tombentley
Copy link
Member

This should now be done.

@gavinking
Copy link
Member

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants