Enhancement: @Getter, support non-final lazy=true and @Setter

lombokissues commented 9 years ago

Migrated from Google Code (issue 556)

lombokissues commented 9 years ago

:bust_in_silhouette: jonl@percsolutions.com :clock8: Aug 08, 2013 at 05:55 UTC

I understand the reasoning behind the final, lazy = true requirement on the @ Getter annotation. However, I have a few problems with it.

The known issue of Context lookup in Eclipse. (slightly off topic here)
The inability to serialize the field without custom serialization.
The inability to override the automatically created object with a setter or even through a constructor.

I have several use cases where I would like to use lazy=true, not to create a final object, but to create a default value if one is not provided. I also like to use the lazy pattern to modularize my code creation. IE, I don't have to create something in the constructor, I create it in a lazy getter instead with the pattern:

private transitive T t;
private static final Object createLock = new Object();

T getT() {
  if (T == null) {
      synchronized(createLock) {
           if (t == null) {
               t = createT();
           }
      }
  }
}

T createT() {
  return new T();
}

public void setT(T newT) {
   if (this.t != newT || (this.t!=null &amp;&amp; !this.t.equals(newT)) {
      synchronized(createLock) {
         if (this.t != newT || (this.t!=null &amp;&amp; !this.t.equals(newT)) {
             this.t = newT;
         }
      }
   }
}

I thought that @ Getter(lazy=true) was my dream come true when I first saw it, but with the issues from eclipse context lookup and inability to serialize the value, I have had to abandon the usage of lazy=true on my getters in most cases.

I would like to see the ability to specify: @Getter(lazyNonFinal=true) @Setter T t;

and generate something similar to the above template, which would resolve the issues with the current version of lazy=true.

lombokissues commented 9 years ago

:bust_in_silhouette: Maaartinus :clock8: Aug 10, 2013 at 15:33 UTC

I also like to use the lazy pattern to modularize my code creation.

Are you aware of DI, especially Guice? In case you're doing what I think, you should consider switching soon.

The test in your setter makes little sense. I guess you're trying something like

!com.google.common.base.Objects(this.t, newT)

but the right part of your condition comes in play only for an object not equal to itself.

I'm curious why your setter shouldn't overwrite the field in case it's equal???

lombokissues commented 9 years ago

:bust_in_silhouette: jonl@percsolutions.com :clock8: Aug 10, 2013 at 20:58 UTC

I am aware of DI and Guice, and there could be usages where Guice and the lazy design pattern would work and they would overlap, however there are reasons not to use Guice or where Guice wouldn't make sense. Perhaps when JSR-330 is incorporated into the core java SDK any need for this pattern will go away, but then again, maybe not.

As for the test, yea that would be what I intended.

As for why it shouldn't overwrite itself in case it's equal? Why should it, what's the point of making a non-operative assignment? If you add additional functionality down the road to support say, Property change support or a notification only when the object changes, then you would need that type of equality check. I have seen at least one request in here to add Property Change support into the core lombok functionality, so I probably had that in mind when writing the test.

lombokissues commented 9 years ago

:bust_in_silhouette: reinierz :clock8: Aug 12, 2013 at 19:50 UTC

The point of making a non-operative assignment is that:

(A) It might still be operative; equal objects may still operate differently. In particular, the premise of: "If the new value is .equals to the old value then your set call is a no-op, but, if they are not .equals(), then T is overwritten" is at least as weird.

(B) .equals isn't free. Why waste a call on it?

The above snippet has all sorts of really weird semantics:

'null' is magic value for the field which means: Actually, the next time someone calls getT(), go calculate the 'lazy expression'. This is not just true after constructing a new object, it's also true after calling .setT(null).
'null', being a magic value, is not something you can return as a 'lazy expression'. Well, you can, but if you do that, the lazy expression will be re-evaluated every time you invoke getT().

Was that intentional? In which case, I'm not sure I understand the underlying use case.

lombokissues commented 9 years ago

:bust_in_silhouette: jonl@percsolutions.com :clock8: Aug 13, 2013 at 20:40 UTC

As for the operative, no-operative assignment and the "weird behavior", a good example of where this type of behavior happens is in the PropertyChangeSupport of java swing. I would say though, that for any code generation by lombok, such wouldn't be necessary in the setter, unless the user wrote the setter themselves, as you would really only need it if you invoke other functionality from the setter. For arguments sake, I would say that if "equals" objects operate differently, then they aren't really "equals", but that's a design choice.

This kind of pattern can be used when creating base classes with common functionality. IE take Apples UIViewController class as an example, it uses this pattern to create the view;

the "get"View operation returns the existing view. If no view is assigned, then it calls the following operations in succession(simplified):

viewWillLoad view = loadView viewDidLoad

then returns the view created from load view.

This pattern provides for a reusable and extendable class. IE UIViewController can be extended in functionality by overriding viewWillLoad, loadView and viewDidLoad. A custom UIViewController that deals with a specific view type can also be reused by swapping in and out of views via the setView operation.

lombokissues commented 9 years ago

End of migration

jvz commented 8 years ago

I'd love this feature for this scenario:

@Getter(lazy = true) private List<String> foo = new ArrayList<>();

to generate something like this:

private List<String> foo;

public List<String> getFoo() {
  if (this.foo == null) {
    this.foo == new ArrayList<>();
  }
  return this.foo;
}

Of course, it could use the double-checked locking or an AtomicReference, but the point here is I'd rather get an empty list than a null reference. It's a common idiom in JAXB classes for instance which are nice to reduce in size with Lombok.

rzwitserloot commented 4 years ago

@jvz this feature is not going to be added because it isn't useful.

Also, it'd mean that calling a getter has a clear observable side effect and that sounds like a very bad idea. The current impl of lazy getter is fine because the field cannot pragmatically be accessed in the first place, and the getter appears to be idempotent. This in contrast to your proposal, where the field remains accessible.

Let's go through all plausible scenarios, because there are many:

You want to have a list, nothing special

Just.. initialize that thing: private List<String> foo = new ArrayList<String>();, what's the big deal? Why are you jumping through these hoops, what possible purpose does it serve?

You want to have a list, but, memory is tight, so specifically for the case that nobody ever interacts with that list in any way or form, do not make it

Tag that getter with (lazy = true), and make sure all your code calls getFoo() and doesn't try to access foo directly.

You want to have a list and be able to modify the variable directly (assign an entire list to it, for example). But I want to default a null value to an empty list in the getter

then your getter should be handwritten, and read: return foo == null ? List.of() : foo;, OR your field should be written: List<String> foo = List.of(); OR List<String> foo = new ArrayList<String>(); depending on what behaviour you are looking for.

Initializing the list inside the getter is a mistake.

I want that, but more efficient

irrelevant; List.of() is incredibly efficient, as it just returns a reference to a single global immutable empty list constant. I need a LOT of evidence that this is somehow a performance issue.

You say 'it is a common idiom in JAXB classes' - can I see an example of this? It sounds like this is a hack to get around an incorrect deserialization; instead the deserialization should either deserialize an empty list there, or you should set it up such that the field starts as List.of(), and is overwritten by JAXB with a freshly created arraylist if it's there, and if not, that initial value stands.

Similar questions to jonl@percsolutions.com - I kinda get what you want here, but it feels like you're written inferior code and that ends all hope that we'd ever allow this feature, as it adds a ton of eyebrow raising issues (such as having a getter with side effects).

Parked - close by 2020-03-01 if no feedback.

projectlombok / lombok