search

neo09 / gwt-platform

Automatically exported from code.google.com/p/gwt-platform
0 stars 0 forks source link

Access modifier for @GenDto #210

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
When generating DTOs using the @GenDto annotation, fields are generated using 
the hardcoded "private" access modifier. This is not not suitable in all cases.

I recommend to evaluate the access modifiers in the source class:
@GenDto
public class Product {
  private Long id;
  String name;
  protected double price;
}

This would generate the DTO:
public class ProductDto {  
  private Long id;
  String name;
  protected double price;

  ...
}

Therefore the code in AnnotationHelper.generateFields() should be changed to 
use Element.getModifiers(), instead of the hardcoded modifier "private".

This issue applies to gwt-platfrom 0.4. It originates from the thread 
http://groups.google.com/group/gwt-platform/browse_thread/thread/7b5e84dd0e88e60
d/

Original issue reported on code.google.com by harald.pehl on 20 Sep 2010 at 2:19

GoogleCodeExporter commented 9 years ago

Original comment by philippe.beaudoin on 20 Sep 2010 at 3:03

GoogleCodeExporter commented 9 years ago 
Ouh crap, there is something like that in the current trunk but I also did not 
use the Modifier enum. I could provide a patch for the GenerationHelper so that 
the modifiers in the generated sources equals the modifier in the @GenX element 
or is this issue already addressed to someone?

Replacement for AnnotationHelper in 0.5:
http://code.google.com/p/gwt-platform/source/browse/src/com/gwtplatform/annotati
on/processor/GenerationHelper.java

Original comment by FloOn...@gmail.com on 20 Sep 2010 at 3:37

GoogleCodeExporter commented 9 years ago 
Go for it !

What's your committer name !?

Original comment by goudreau...@gmail.com on 20 Sep 2010 at 4:10

GoogleCodeExporter commented 9 years ago 
I think that should be floonweb.

Original comment by FloOn...@gmail.com on 21 Sep 2010 at 7:49

GoogleCodeExporter commented 9 years ago 
I just assigned you this issue :D Try to work on a branch with the issue number 
or a clone if it's going to be breaking change.

Cheers,

Original comment by goudreau...@gmail.com on 21 Sep 2010 at 12:10

GoogleCodeExporter commented 9 years ago 
Great, thanks Christian :) I'll work with my clone.

What do you think about following to solve the problems with final fields:

Use the @Final annotation on fields when you want to set the final value by 
passing a parameter to constructor.

 Code:
 @GenDto
 public class Product {
   @Final private String myAge;
   public String myName;
 }

 Generates:
 public class ProductDto {
   private final String myAge;
   public String myName;

   public MyClass(String myAge, String myName) {
     this.myAge = myAge;
     this.myName = myName;
   }

   ....
}

Final fields without the @Final annotation will generate following:
Code:
@GenDto
public class Product {
  private final String myAge = "20";
  public String myName;
}

Generates:
public class ProductDto {
  private final String myAge = "20";
  public String myName;

  public MyClass(String myName) {
  this.myName = myName;
}

greetings Flori

Original comment by FloOn...@gmail.com on 22 Sep 2010 at 6:56

GoogleCodeExporter commented 9 years ago 
[deleted comment]
GoogleCodeExporter commented 9 years ago 
Or you think it is not required to initialize final fields subsequently?

Original comment by FloOn...@gmail.com on 22 Sep 2010 at 7:03

GoogleCodeExporter commented 9 years ago 
Number two is better, no ? though Getter and setter are missing or it was to 
simplify the example ? :D

Original comment by goudreau...@gmail.com on 22 Sep 2010 at 11:26

GoogleCodeExporter commented 9 years ago

Original comment by philippe.beaudoin on 22 Sep 2010 at 1:40

GoogleCodeExporter commented 9 years ago

Original comment by FloOn...@gmail.com on 22 Sep 2010 at 11:52

GoogleCodeExporter commented 9 years ago

Original comment by goudreau...@gmail.com on 23 Sep 2010 at 10:20

GoogleCodeExporter commented 9 years ago 
If u want to set a final field over the constructor the according object can 
not have an empty constructor for serialization. So I think that in most cases 
the setting of final fields over the constructor is meaningless and therefore 
shouldn't be possible for @GenX annotations. Do you agree?

Original comment by FloOn...@gmail.com on 24 Sep 2010 at 10:17

GoogleCodeExporter commented 9 years ago 
I agree. I would even have said: no final for DTOs since final fields are not 
serialized. (Maybe allow for public final static fields for constants, but they 
can store them elsewhere, a DTO is not the best place for this.)

Original comment by philippe.beaudoin on 24 Sep 2010 at 6:50

GoogleCodeExporter commented 9 years ago 
Patch available: http://codereview.appspot.com/2283041/

I would be happy if you test the generation in more detail and all kind of 
combinations.

Original comment by FloOn...@gmail.com on 27 Sep 2010 at 8:31

GoogleCodeExporter commented 9 years ago 
Created a new clone with changes:
http://code.google.com/r/floonweb-annotationmodifiers/source/browse

Please ignore the codereview mentioned above.

Original comment by FloOn...@gmail.com on 29 Sep 2010 at 8:29

GoogleCodeExporter commented 9 years ago 
New code review available at: http://codereview.appspot.com/2317041

Original comment by FloOn...@gmail.com on 29 Sep 2010 at 8:37

GoogleCodeExporter commented 9 years ago 
Merged with trunk.

Florian, can you please update 
http://code.google.com/p/gwt-platform/wiki/BoilerplateGeneration with details 
of new features.

Original comment by bren...@doherty.net.nz on 7 Oct 2010 at 6:43

GoogleCodeExporter commented 9 years ago 
Reopening until doc is updated.

Original comment by philippe.beaudoin on 7 Oct 2010 at 4:26

GoogleCodeExporter commented 9 years ago 
http://code.google.com/p/gwt-platform/wiki/BoilerplateGeneration updated 
(section @Optional & Modifiers and Constants).

Original comment by FloOn...@gmail.com on 10 Oct 2010 at 4:03

GoogleCodeExporter commented 9 years ago

Original comment by FloOn...@gmail.com on 10 Oct 2010 at 4:09