Feedback and suggestions

[GHad]

Hi everyone from the Project Lombok User Group!


I really hope, this is the right place to share my opinion and my
thoughts about this great java enhancement. I looked around but
couldn't find a proper place to send over my feedback.

Let me introduce myself: I'm Gerhard Balthasar, 28 years old and
Software-Engineer at a small software and network company in southern
Germany. So please apologize any language mistakes I may make. I'm
programming Java for more than 6 years now while studying and at work.

I played around with the lib for some hours now, I think it's great
and just want to share some feelings. I'm not deep into code (yet) and
some of this maybe already be done or planed.

Anyway, let's roll...

@Getter and @Setter
- You may create @NoSetter and @NoGetter annotations, this would help
a lot for @Data (see below)
- You should be carefull about arrays and collections. Creating a

public void setFoo(Foo[] foo) { this.foo = foo; }

setter exposes internal structure. See FindBugs for details. This
should be considered as capsulation flaw. Either generate no setters/
getters or find a safe way, e.g. by returning an unmodifiable
collection from the setter:

public Collectiony<?> getFoo() { return
Collections.unmodifieableCollection(foo); }

- Adding a @Has annotaion to a field foo would create a

public boolean hasFoo() { return this.foo != null; }

method for Objects, but not for primitives. Give same AccessLevel
option as for getters/setters to it.

@EqualsAndHashCode
- As far as I understand this annotation: if you don't extend any
class, use callSuper=false, else callSuper=true. So this can be
reflected. Why the clutter for the developer?

@ToString
- I would twist the default of incudeFieldNames, so it defaults to
true. It may be lengthy, but imho it's more annoying to look up a
class and counting down the fields to find out the name of the field
for the value. This would also help for @Data, as this one is often
used in situations, where there are many fields and one cannot
influence the behavoir of @ToString except by declaring it explicitly.

@Data
- Transient fields are not excluded for the creation of getters/
setters, but should be. Else, you have no chance to not create them,
but by creating clutter. Either by annotating them with @Setter
(AccessLevel.PRIVATE)/... or by creating an empty private method. Both
bad.
- As an alternative introduce @NoSetter/@NoGetter

@Synchronized
- You may switch to Lock/ReentrantLock (java.util.concurrent.locks)
instead of using synchronized and a dummy object array

@Cleanup and @SneakyThrows
- Not looked deeper into.

More ideas:

- Seperate @Builder and @Fluent, so that @Fluent can be used at any
class for any method: The additionally created methods for fields just
need to call the corresponding setters/gettern and return this instead
of void. Moreover, @Fluent would also convert any void methods return
type to the class's type and return this.

- What about validation? When you create a constructor for final
fields or a static method for generic instance creation. Even with
setters, how can the developer validate its argumens? I just want two
things: an easy way to make the default validations: not null, not
empty (for Strings: does not equal ""), is greater than, in a range,
exists, ... which throws away a validation exception, if the object is
not valid. And on the other hand a way to
enforce my own validator to kick in. But wait... There is already a
reference implementation by the Hibernate team for the Bean Validation
Framework JSR 303 (https://www.hibernate.org/412.html). So why not
incorparate this and make validation a non-typer for the developer:

[code]
import java.io.*;
import java.lang.annotation.*;

import javax.validation.*;
import javax.validation.constraints.*;

import validation.created.AreaVanilla.*;

/**
*
* FOR DEMONSTRATION ONLY. NO RESPONSIBILTIY FOR ANYTHING, COPYLEFT
*
* @author g...@gmx.de (Gerhard Balthasar)
*
*/

@Data
public class Area {

@NotNull
@Size(min = 1, message = "must not be ''")
private final String name;

@Min(-180)
@Max(180)
private final long langitude, latitude;

@Size(min = 1, message = "must not be ''")
private String country;

@MustBeNull(message = "must be null")
private Object mustBeNull;

// --- Own validation implementation (can be reused & externalized)

@Retention(value = RetentionPolicy.RUNTIME)
@Constraint(validatedBy = NullValidator.class)
public @interface MustBeNull {
String message();

java.lang.Class<?>[] groups() default {};

java.lang.Class<?>[] payload() default {};
}

public static class NullValidator implements
ConstraintValidator<MustBeNull, Object> {

@Override
public void initialize(MustBeNull constraintAnnotation) {

}

@Override
public boolean isValid(Object value, ConstraintValidatorContext
constraintValidatorContext) {
return value == null;
}

}

}

[/code]

So, if the developer uses the JSR 303 Hibernate Reference
implementation, this can transform to the (manually implemented)
working class:

[code]
/**
*
* FOR DEMONSTRATION ONLY. NO RESPONSIBILTIY FOR ANYTHING, COPYLEFT
*
* @author g...@gmx.de (Gerhard Balthasar)
*
*/

public class AreaVanilla {

// --- For simple tests...

public static void main(String[] args) {
AreaVanilla area = new AreaVanilla("", -200, 42);
area.setMustBeNull("Never reached, but would throw
ValidationException anyway...");
}

// --- Original content

@NotNull
@Size(min = 1, message = "must not be ''")
private final String name;

@Min(-180)
@Max(180)
private final long langitude, latitude;

@Size(min = 1, message = "must not be ''")
private String country;

@MustBeNull(message = "must be null")
private Object mustBeNull;

@Retention(value = RetentionPolicy.RUNTIME)
@Constraint(validatedBy = NullValidator.class)
public @interface MustBeNull {
String message();

java.lang.Class<?>[] groups() default {};

java.lang.Class<?>[] payload() default {};
}

public static class NullValidator implements
ConstraintValidator<MustBeNull, Object> {

@Override
public void initialize(MustBeNull constraintAnnotation) {

}

@Override
public boolean isValid(Object value, ConstraintValidatorContext
constraintValidatorContext) {
return value == null;
}

}

// --- Added by @Data transformation (Constructor & setters modified)

// private constructor for JaxB and such (should be added imho)
public AreaVanilla() {
this.name = null;
this.langitude = 0;
this.latitude = 0;
}

// public constructor for class users/factory methods
public AreaVanilla(final String name, final long langitude, final
long latitude) {
this.name = name;
this.langitude = langitude;
this.latitude = latitude;
// now validate me, throws ValitionException if invalid
$validate(this);
// create the validation object
$VALIDATION_OBJECT = new AreaVanilla(name, langitude, latitude,
true);
}

// private constructor for inner validation object creation (without
// validation, so no endless recursion)
private AreaVanilla(final String name, final long langitude, final
long latitude, final boolean inner) {
this.name = name;
this.langitude = langitude;
this.latitude = latitude;
}

public String getName() {
return name;
}

public long getLangitude() {
return langitude;
}

public long getLatitude() {
return latitude;
}

public String getCountry() {
return country;
}

public void setCountry(String country) {
// synchronize for validation
$VALIDATION_LOCK.lock();
// assume invalid
boolean $valid = false;
try {
// set validations object property to new value
$VALIDATION_OBJECT.country = country;
// throw ValidationException upon invalid validation object
$validate($VALIDATION_OBJECT);
// only reached if validation succeeds
$valid = true;
} finally {
// if validation failed, restore previous value on validation
object
if (!$valid) {
$VALIDATION_OBJECT.country = this.country;
}
// release the synchronization lock
$VALIDATION_LOCK.unlock();
}
// only ever reached if valid
this.country = country;
}

public Object getMustBeNull() {
return mustBeNull;
}

public void setMustBeNull(Object mustBeNull) {
$VALIDATION_LOCK.lock();
boolean $valid = false;
try {
$VALIDATION_OBJECT.mustBeNull = mustBeNull;
$validate($VALIDATION_OBJECT);
$valid = true;
} finally {
if (!$valid) {
$VALIDATION_OBJECT.mustBeNull = this.mustBeNull;
}
$VALIDATION_LOCK.unlock();
}
this.mustBeNull = mustBeNull;
}

// --- Added by JSR303 transformation

/**
* JSR 303 Validator
*/

private static final Validator $VALIDATOR;

/**
* Inner validation object<br>
* Used to ensure the objects state NEVER changes until validated
*/

private AreaVanilla $VALIDATION_OBJECT;

/**
* This lock synchronizes validation upon multiple thread access
*/

private Lock $VALIDATION_LOCK = new ReentrantLock(true);

/**
* Creates the instance of Validator by using the appropriate factory
*/

static {
ValidatorFactory factory = Validation.buildDefaultValidatorFactory
();
$VALIDATOR = factory.getValidator();
}

/**
*
* Validates and throws a ValidationException, if there is any
validation
* contraint violations on the object to validate
*
* @param toValidate
* the object to validate
* @throws ValidationException
* if there are constraint violations, constructs a
* ValidationException of the constraints and sets
StackTrace
* origin at the original method that called this method
*/

private void $validate(AreaVanilla toValidate) throws
ValidationException {
Set<ConstraintViolation<AreaVanilla>> constraintViolations =
$VALIDATOR.validate(toValidate);
if (!constraintViolations.isEmpty()) {
StringBuilder sb = new StringBuilder();
boolean first = true;
for (ConstraintViolation<AreaVanilla> constraintViolation :
constraintViolations) {
if (first) {
first = false;
} else {
sb.append("; ");
}
String message = constraintViolation.getMessage();
Path propertyPath = constraintViolation.getPropertyPath();
Iterator<Node> iterator = propertyPath.iterator();
while (iterator.hasNext()) {
Node node = iterator.next();
String name = node.getName();
// FIXME why is first item always 'null' and the second item
// is the field name?
if (name != null && !"".equals(name.trim()) && !"null".equals
(name.trim())) {
sb.append(name).append(" ");
}
}
sb.append(message);
}
ValidationException e = new ValidationException(sb.toString());
StackTraceElement[] stackTrace = e.getStackTrace();
StackTraceElement[] newStackTrace = new StackTraceElement
[stackTrace.length - 1];
System.arraycopy(stackTrace, 1, newStackTrace, 0, stackTrace.length
- 1);
e.setStackTrace(newStackTrace);
throw e;
}
}

}

[/code]

Just try this code yourself. The result is completly independant of
Lombok and completly re-creatable at any time. So Lombok just needs to
create the validation calls and exception creation. The rest is
transparently done by JSR 303. No clutter for the developer, thread-
safe and state-safe runtime validation based upon (future) standards.

Well, I hope you enjoyed my feedback and I would love to read yours.

Greetz,
GHad

[Reinier Zwitserloot]

This is the right place for feedback :)

replies inline.

On Aug 18, 1:18 am, GHad <g...@gmx.de> wrote:
> - You should be carefull about arrays and collections. Creating a
>
> public void setFoo(Foo[] foo) { this.foo = foo; }
>

returning an immutable view would be a surprise. That brings lombok
firmly into the too much magic camp, I think. I'm empathetic to the
issue; after all, if you want to have an immutable object that
contains a list, it won't be, if people construct them with just your
average (non-Collections.unmodifiableList()-protected) ArrayList.

However, there are a whole host of issues in automating this in
lombok:

(1) - arrays: Screw em. People who want to use arrays have to deal
with the consequences. I -really- don't feel like burdening everyone's
understanding of how lombok's transformations work by a bunch of
special rules for arrays.

(2) - collections API: We'd have to hard code the rules in. I don't
like that. What if people prefer javolution? What if they prefer
google collections API? Speaking of the google collections API, if you
really want immutable lists, you can just set the type of your fields
to the g.c ImmutableList, that'll help a little bit. Any solution here
would have to farm out the job of returning an immutable copy to the
object in question. I'm not so much against hardcoding for java.util
(it's java.util! It's allowed to be special), but I'm more worried
about the magic factor: How can one know that java.util.List, Set, and
Maps are protected via Collections.unmodifiable*, but everything else
isn't? You wouldn't know unless you sift through the docs. That's not
good.

> - Adding a @Has annotaion to a field foo would create a
>
> public boolean hasFoo() { return this.foo != null; }

I'm not convinced. Lombok aims to eliminate boilerplate. It's not
designed to become a new programming language. If you want a hasFoo()
method, just add it to the code. It's a one-liner, not too bad.

> @EqualsAndHashCode
> - As far as I understand this annotation: if you don't extend any
> class, use callSuper=false, else callSuper=true. So this can be
> reflected. Why the clutter for the developer?

That might change in the future. It's again the 'do the magic thing'
issue. If you don't extend anything / you extend java.lang.Object,
then you don't need a callSuper. If you do supply callSuper=true, the
code will not even compile, as that's just stupid. However, if you DO
extend something else, the problem has become more complex than lombok
can handle.

Not calling super can be understandable, especially if you're
extending a flag class or one that simply has no fields. Then it would
be the right thing to do. But, calling super only works if the super
implementation of equals is 'subclass-ready'. If people write their
equals like so:

if ( other.getClass() != MyClass.class ) return false;

then your equals() method just won't work. It HAS to be written with
if ( other.getClass() != this.getClass() ) return false, like lombok
does. Lombok cannot know how the parent's equals has been implemented,
so it warns you. The warning is essentially lombok telling you: I
don't know enough to generate the equals method. I need you to
explicitly tell me if you want me to call super or not.

Once lombok gets the ability to do type introspection, it can detect
if the parent's equals and hashCode implementations are generated by
lombok (and therefore, totally safe for chaining). The default action
can then be to call super and not to emit that warning.

> - I would twist the default of incudeFieldNames, so it defaults to
> true. It may be lengthy, but imho it's more annoying to look up a
> class and counting down the fields to find out the name of the field
> for the value.

The original idea behind ToString is that the printed string is all
you need to construct a new one, provided you stick 'new' in front of
it. Which doesn't really hold up - strings aren't even escaped and
quoted. So, I think you're right about this. It could be smarter about
it and only default to includeFieldNames if you have more than 1 or 2,
but then we get back to the 'too much magic' problem. So, let's go
with includeFieldNames true by default, no matter how many fields you
have. Can you file a ticket for this? Thanks.

> This would also help for @Data, as this one is often
> used in situations, where there are many fields and one cannot
> influence the behavoir of @ToString except by declaring it explicitly.

Agreed.

>
> @Data
> - Transient fields are not excluded for the creation of getters/
> setters, but should be. Else, you have no chance to not create them,
> but by creating clutter. Either by annotating them with @Setter
> (AccessLevel.PRIVATE)/... or by creating an empty private method. Both
> bad.
> - As an alternative introduce @NoSetter/@NoGetter

Eh. Feels like too many annotations to me. I could extend AccessLevel
to include .DONTGENERATE but that's even worse (nothing is not an
access level, of course). It's a problem though. Is there a way out of
this that's nicer? Perhaps add exclude="" to @Data itself. The reason
lombok doesn't cater to these situations is that it's not what lombok
was for. Any class with transient fields is probably well beyond the
level of 'trivial little structy class' which is what @Data was meant
for. So I'm not convinced lombok needs to cater to this case. How
often do you use transient fields? I can count on one hand how often
I've used them, and in all cases, @Data wouldn't really fit for that
class.

>
> @Synchronized
> - You may switch to Lock/ReentrantLock (java.util.concurrent.locks)
> instead of using synchronized and a dummy object array

Is there an advantage to doing that, though? Once lombok gains the
ability to introspect on types, we could detect that you're trying to
synchronize on a lock, and lock the lock instead.

>
> More ideas:
>
> - Seperate @Builder and @Fluent, so that @Fluent can be used at any
> class for any method: The additionally created methods for fields just
> need to call the corresponding setters/gettern and return this instead
> of void. Moreover, @Fluent would also convert any void methods return
> type to the class's type and return this.

I'll have to think further on this. @Builder in particular is complex
as hell. Is there something like a de facto builder implementation? Is
the builder class an inner class of the object to be constructed, or
separate? Do you construct a new builder, or do you call a static
method on the to-be-built class? What's it called? What's the method
called to go from builder to constructed object? done()? make()? How
do you handle mandatory fields? Stuff them in the constructor of the
builder / that static method that makes one, or do you just build them
as usual, with fluent methods, and emit a runtime error if you forget
the mandatory fields? How much, if any, of these questions need to
become configurable via annotation parameters?

If there are no suitable answers, then lombok should just pick as best
an answer as it can and get a @Builder out there, but we can
deliberate on exactly how to roll with it.

>
> - What about validation? When you create a constructor for final
> fields or a static method for generic instance creation. Even with
> setters, how can the developer validate its argumens? I just want two
> things: an easy way to make the default validations: not null, not
> empty (for Strings: does not equal ""), is greater than, in a range,
> exists, ... which throws away a validation exception, if the object is
> not valid.

NotNull already exists in the git trunk, which triggers off of any
annotation named NotNull and NonNull (there are a bunch floating out
there. Hopefully they'll all coalesce on JSR305's annotations). But
such a system sounds like quite an undertaking. I'm not sure the
burden of fielding a complex validation framework is really going to
help make your code more readable and less boilerplate-y. Is this:

private @EnsureNotEmpty String name;

really that much of an improvement over:

private String name;

public void setName(String name) {
if ( name == null ) throw new NPE();
if ( name.length() == 0 ) throw new ValidationException("name must
not be empty.");
}


The answer is of course: Yes, it's more readable, and less
boilerplatey, but by how much of a factor, and at what cost? There are
the usual slate of open questions, too: Should this validation system
be pluggable? Should, in fact, the entirety of @Data/@Setter/@Getter
be pluggable (should it auto-discover template generators and call
into them to generate the setter and getter given the annotations on
the type and the field in question?)

> enforce my own validator to kick in. But wait... There is already a
> reference implementation by the Hibernate team for the Bean Validation
> Framework JSR 303 (https://www.hibernate.org/412.html). So why not
> incorparate this and make validation a non-typer for the developer:

Yeah, I was thinking of that too. Either lombok becomes pluggable, or
lombok just becomes JSR 303 compatible. It's a good idea. Feel free to
file an issue for it, but give it priority low - as cool as this is,
it can't hold a candle to @Delegate and some of the other ideas in the
pipeline, I think. Or, better yet, get hacking on this yourself! I'll
accept a patch that's JSR 303 compatible.

>
> Well, I hope you enjoyed my feedback and I would love to read yours.
>

Thanks for taking an interest, Gerhard.

[GHad]

Thanks for your answer,

Sorry for taking a short break, I tried much code and had many things
to do last days. And I wanted to provide an usefull answer, replies
inline.

> > - You should be carefull about arrays and collections. Creating a
>
> > public void setFoo(Foo[] foo) { this.foo = foo; }
>
> returning an immutable view would be a surprise. That brings lombok
> firmly into the too much magic camp, I think. I'm empathetic to the
> issue; after all, if you want to have an immutable object that
> contains a list, it won't be, if people construct them with just your
> average (non-Collections.unmodifiableList()-protected) ArrayList.
>
> However, there are a whole host of issues in automating this in
> lombok:
> (1) - arrays: Screw em.

Agree. Sometimes even arrays are usefull, e.g. byte[], but normally
those must be exposed...

>
> (2) - collections API: We'd have to hard code the rules in. I don't
> like that. What if people prefer javolution?

Any other collection API transforms with the usual rules to getters/

setters, but as you wrote:

> (it's java.util! It's allowed to be special), but I'm more worried
> about the magic factor: How can one know that java.util.List, Set, and
> Maps are protected via Collections.unmodifiable*, but everything else
> isn't? You wouldn't know unless you sift through the docs. That's not
> good.

Yes, you're right. The developer needs a way to declare the behavoir
somewhere, if desired. A simple solution would perhaps be an @Getter
attribute

@interface Getter {

//...

boolean immutable() default false;

}

- For java.util.* special implementation, e.g.

@Getter(immutable=true)
private List<String> unexposed;

public List<String> getUnexposed() {
return Collections.unmodifiableList(unexposed);
}

- Would not allow setters to be created
- For primitives no additional action taken
- Else return a deep clone at the getter method

This shouldn't be too magic and is easily describeable at the docs/
examples and would extend to a more flexible usage for getters. What
do you think?

>
> > - Adding a @Has annotaion to a field foo would create a
>

> I'm not convinced. Lombok aims to eliminate boilerplate. It's not
> designed to become a new programming language. If you want a hasFoo()
> method, just add it to the code. It's a one-liner, not too bad.

Yeah. Ok, as it's a oneliner and not standard for APIs and such,
forget about this one.

> > @EqualsAndHashCode
> ...

> Once lombok gets the ability to do type introspection, it can detect
> if the parent's equals and hashCode implementations are generated by
> lombok (and therefore, totally safe for chaining). The default action
> can then be to call super and not to emit that warning.

Ok, looking forward to it. As I noticed but not read yet, there is a
discussion on track about this. So I may participate there.

> The original idea behind ToString is that the printed string is all
> you need to construct a new one, provided you stick 'new' in front of
> it. Which doesn't really hold up - strings aren't even escaped and
> quoted. So, I think you're right about this. It could be smarter about
> it and only default to includeFieldNames if you have more than 1 or 2,
> but then we get back to the 'too much magic' problem. So, let's go
> with includeFieldNames true by default, no matter how many fields you
> have. Can you file a ticket for this? Thanks.

I'll do after posting the answer.

> > @Data
> > - Transient Fields are not excluded ...

> > - As an alternative introduce @NoSetter/@NoGetter
>
> Eh. Feels like too many annotations to me. I could extend AccessLevel
> to include .DONTGENERATE but that's even worse (nothing is not an
> access level, of course). It's a problem though. Is there a way out of
> this that's nicer? Perhaps add exclude="" to @Data itself. The reason
> lombok doesn't cater to these situations is that it's not what lombok
> was for. Any class with transient fields is probably well beyond the
> level of 'trivial little structy class' which is what @Data was meant
> for. So I'm not convinced lombok needs to cater to this case. How
> often do you use transient fields? I can count on one hand how often
> I've used them, and in all cases, @Data wouldn't really fit for that
> class.

BTW, working with EJBs and JaxB I have transient fields quite often.
Not that much but I need them now or then. But the point is: When
using @Data my classes often have only a getter or a setter for a
field. Especially for Lists/Maps, etc. the above mentioned 'immutable'
attribute would be enough, but for any other field type I need a way
to avoid the creation of a setter/getter. I really think, Lombok
should handle this case. But I agree with you:

- Adding an exclude list on @Data would not be enough, because you
would need a list for getters and for setter. Much typing. Bad.
- Extending AccessLevel is bad also, I agree.
- @NoSetter/@NoGetter, which are additional annotaion with no other
value left. Also bad.
- Transient fields are neither a good way, although lombok already
reacts on them when using @EqualsAndHashCode. But extending this to
@Getter/@Setter would prevent Setter/Getter method creation and

normaly transient fields need access methods also.

Conclusion: Bad. What's left then?

Changing the behaviour of @Datas Setter/Getter creation would be an
option:

- Create Setter and Getter if neither @Setter nor @Getter is annotated
at a field or @Setter and @Getter are both anotated
- Create only Setter if @Setter is annotated
- Create only Getter if @Getter is annotated

In other words: create only the method type that is declared, but
default to both annotations declared, if not annotated. No additional
clutter but again, the behavoir is simple declared by the annoations
the developer writes down. I think this can be considered a logical
behavior in my eyes and easy to understand.

Would this solve the problem?

> > @Synchronized
> ...

> Is there an advantage to doing that, though? Once lombok gains the
> ability to introspect on types, we could detect that you're trying to
> synchronize on a lock, and lock the lock instead.

Yes there are multiple advantages for using a ReentrantLock over an
object array and synchronized:

- The acquireing of the lock can have a timeout: tryLock(long time,
TimeUnit unit);
- If the developer knows the name of the lock variable (he can declare
the lock field or use the variable created) he is able to use
Conditions for Consumer/Producer like problems and all other sort of
notify scenarios (see http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/locks/Condition.html).
- With fair=true the order of the waiting threads is preserved when
executed after release of the lock
- A synchronize on ReentrantReadWriteLock can be used to synchronize a
method explicitly for read or write thus speeding up parallel access

Well but I see... packing this all into different annotiotions is
huge. Well, lombok could again use an attribute:

@interface Synchronized {

//...

LockBehavior lock() default LockBehavior.SYNCHRONIZED;

}

so the original way kicks in. Additional enums would be:

- REENTRANT: Use a ReentrantLock, usefull if using conditions
- FAIR: Use a ReentrantLock with fair=true, usefull if using
conditions
- TRY: Only try to obtain lock, use tryLock() method
- TIMEOUT: which would need additional attributes for the time and
unit though
- READ: use ReentrantReadWriteLock().readLock()
- WRITE: use ReentrantReadWriteLock().writeLock()
- THIS: synchronized(this) instead of synchronized($lock)

Well, I also found negatives by google:
http://book.javanb.com/java-concurrency-in-Practice/ch13lev1sec4.html
(Java Concurrency in Action, Chapter 13.4)

You decide... But I think using Locking over synchronized is supirior
because of the additionally options for the developer, even better
with the extended annotation

> > More ideas:
>
> > - Seperate @Builder and @Fluent, so that @Fluent can be used at any

> ...

> If there are no suitable answers, then lombok should just pick as best
> an answer as it can and get a @Builder out there, but we can
> deliberate on exactly how to roll with it.
>

As for @Builder: I need to read more messages from the board and some
prototyping before proposing feedback or an implementation. But I
wanted to concentrate on @Fluent here as an independant Annotation.
@Builder would only incorparate @Fluid like @Data includes @Setter/
@Getter for example. If @Fluent itself would include @Getter and
@Setter, the main advantage is usability for every class out of the
box:

@Fluent
class Example {
String name;
public void foo() { System.out.print("foo"); }
}

transforms to:

class Example {
String name;
public Example foo() { System.out.print("foo"); return this; }

public String getName() { return name; }
public void setName(String name) { this.name = name; }

public String name() { return this.getName(); }
@SuppressWarning("hiding")
public Example name(String name) { this.setName(name); return this; }

}

Would preserve original setter/getter syntax which could be important
for reflection, etc. I know more issues will arise about naming, deep
nesting, overwritten methods and such, but I think it's

worth it and @Builder could automaticly reuse this functionality.

> > - What about validation?
> ...

> NotNull already exists in the git trunk, which triggers off of any
> annotation named NotNull and NonNull (there are a bunch floating out
> there. Hopefully they'll all coalesce on JSR305's annotations).

You may consider to remove @NotNull @NonNull from the release again,
if JSR303 compatibility is reached?

> such a system sounds like quite an undertaking. I'm not sure the
> burden of fielding a complex validation framework is really going to
> help make your code more readable and less boilerplate-y. Is this:

Yes, it makes it declerative, thus readable and understandable for
everone looking at the fields, not at the implementation of methods.
There are also less tests to write, if automatic validation is

provided by lombok on which developers and tester can rely. I'm
currently working on possibilities to achieve a flexible yet clear and
easy to use way for JSR303 support while respecting different

contexts and requirements from the developers point of view. I think
I'll file in an issue when requirements are clear.

> The answer is of course: Yes, it's more readable, and less
> boilerplatey, but by how much of a factor, and at what cost? There are
> the usual slate of open questions, too: Should this validation system
> be pluggable? Should, in fact, the entirety of @Data/@Setter/@Getter
> be pluggable (should it auto-discover template generators and call
> into them to generate the setter and getter given the annotations on
> the type and the field in question?)

Yes, I think a plugable solution would be best to encourage develpers
to code their own annotations and transformations. Also other APIs
would be able to support Lombok with own annotations and create
boilerplate code for the developer. Plus many internal calls can be
made plugable too, I think although I'm not into code yet. The point
is, if thinking of transformations in a form of aspects, a plugable
system would fit perfectly. Combining transformation with aspect gives
a @Tanspect. Using a similar system like defining Validators for
JSR303 you may come up with something like the follwing classes, which
are i simplified idea:

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.ANNOTATION_TYPE)
public @interface Transpect {

Class<? extends TranspectInterceptor<?>>[] interceptedBy();

Class<? extends Annotation> annotation() default Transpect.class;

}

public interface TranspectInterceptor<T extends Annotation> {

public void transform(List<TransformationPoint<T>> points,
TransformationContext context);

}
r
Along with some other classes this would basicly allow to define an
interception fo the transfomation process for one annotation and which
class to process the transpect: Using this for my infamous

@Has annotation I would write something like this:

@Transpect(annotation = Getter.class, interceptedBy =
HasTransformer.class)
@Retention(RetentionPolicy.RUNTIME)
public @interface Has {
AccessLevel accessLevel() default AccessLevel.PUBLIC;
}

public static class HasTransformer implements
TranspectInterceptor<Has> {

@Override
public void transform(List<TransformationPoint<Has>> points,
TransformationContext context) {
TransformationClass target = context.targetClass();
for (TransformationPoint<Has> point : points) {
switch (point.elementType()) {
case FIELD:
Field field = point.field();
Has annotation = point.transpect();
createHas(target, field, annotation);
break;
default:
}
}
}

private void createHas(TransformationClass target, Field field, Has
annotation) {
// TODO: respect accessLevel()
String method = "public has$upperName() { return $name != null; }";
String name = field.getName();
String firstUppercaseName = name.substring(0, 1).toUpperCase() +
name.substring(1);
method = method.replace("$upperName", firstUppercaseName).replace
("$name", name);
target.addMethod(method);
}
}

@Data
public static class Foo {

@Has
private String name = null;

}

// --- transforms to

public static class FooVanilla {

private String name = null;

public String getName() {
return name;
}

public void setName(String name) {
this.name = name;
}

public boolean hasName() { return name != null; };

}

This is just a vast exmaple of how I think a plugable system could
work. It's surely much too short-sighted but I think you're getting
the idea.

> > enforce my own validator to kick in. But wait... There is already a
> > reference implementation by the Hibernate team for the Bean Validation
> > Framework JSR 303 (https://www.hibernate.org/412.html). So why not
> > incorparate this and make validation a non-typer for the developer:
>
> Yeah, I was thinking of that too. Either lombok becomes pluggable, or
> lombok just becomes JSR 303 compatible. It's a good idea. Feel free to
> file an issue for it, but give it priority low - as cool as this is,
> it can't hold a candle to @Delegate and some of the other ideas in the
> pipeline, I think. Or, better yet, get hacking on this yourself! I'll
> accept a patch that's JSR 303 compatible.

I'll do... At least I'll try, I'll file in an issue as written above
and then I'm going the get this done simple, easy and clear. But: This
is my first open source project I want to participate further. Can you
give me some advise about do's and don'ts and where to start with git,
where information on the build process is, etc... Is there a
developers faq for contributors or a how to start for dummies :-) out
there?

Greetz,
GHad

[Reinier Zwitserloot]

Replies inline.

On Aug 23, 12:25 am, GHad <g...@gmx.de> wrote:
> - For java.util.* special implementation, e.g.
>
> @Getter(immutable=true)
> private List<String> unexposed;
>

No, it would have to make a deep clone in the constructor, otherwise
it wouldn't be immutable (you could pass in a mutable list during
construction, and mutate it outside of your class with the @Getter
(immutable=true).

In order to operate correctly, @Getter needs to know from an object:

A) Are you already immutable?

B) If you aren't, how do I clone you?

C) Do you have a 'create immutable wrapper' type of feature, or can I
clone you into an immutable object?


Unfortunately absolutely none of that is available unless you hard
code it for every type. Which still strikes me as unacceptable.

As this feature isn't nearly as elegant as some of the other lombok
ideas, it'll have to move to the back of the queue.

I agree with you that, if we find a solution to how to answer these 3
questions, the addition of an explicit parameter on @Getter reduces
the magic factor enough for it to be an acceptable addition to lombok.

>
> - @NoSetter/@NoGetter, which are additional annotaion with no other
> value left. Also bad.

It might be acceptable if @NoSetter and @NoGetter become sub-
annotations of @Data. So:

public @interface Data {
public @interface NoSetter {}
public @interface NoGetter {}
}

That seems a fair compromise between sticking too much crud in the
lombok package and offering this flexibility, which does have its
place. File a ticket for it?

>
> Yes there are multiple advantages for using a ReentrantLock over an
> object array and synchronized:
>
> - The acquireing of the lock can have a timeout: tryLock(long time,
> TimeUnit unit);

Yup, but, as you said, how would you specify the timeout and timeunit?
parameters of the annotation? Meh. This is again leaning not so much
to eliminating boilerplate but more to creating a new language, which
is not what lombok is designed to do, or at least, if we're going to
go there, we should get it right, and add e.g. closures instead of
adding all these separate bits which will wind up being one giant
incoherent ball.

I'm thinking we should solve this instead by adding closures via
lombok. Which is a major project and won't happen overnight.

Sticking the options on lombok's annotations also means it's not
extensible at all, and this again strikes me as a patchy mess and not
as an elegant solution.

>
> [@Fleunt is] worth it and @Builder could automaticly reuse this functionality.
>

Probably. We'll tackle this when @Builder is added, which might be a
while. Again, many of these proposals have plenty of less-than-elegant
issues, and are often not as compelling as some of the things already
proposed (such as @Delegate) that are completely elegant, so we'll
work on those first. We're just 2 guys spending some of our spare time
on this project!

>
> You may consider to remove @NotNull @NonNull from the release again,
> if JSR303 compatibility is reached?
>

No. Just because JSR303 exists does not mean I'm going to force it
down everybody's throat. You have a @NonNull/@NotNull annotation?
Lombok will honour it.

Of course, if people -choose- to use JSR303, lombok should honour that
too.

>
> I think
> I'll file in an issue [on JSR303] when requirements are clear.
>

Yes, please do, and thanks.

>
> Yes, I think a plugable solution would be best to encourage develpers
> to code their own annotations and transformations. Also other APIs
> would be able to support Lombok with own annotations and create
> boilerplate code for the developer.

But lombok is already pluggable. Okay, the @Setter/@Getter annotations
are not, but before putting in the considerable effort, lets first see
how many other projects are willing to invest the time. If a few are,
then clearly making Setter/Getter pluggable should result in lots of
happy faces and a lot more projects taking the effort. If nobody does,
well, as you can see, we've got enough issues and ideas on the
backburner to last us for a _DECADE_, so we need to pick and choose
what to work on.

> [Transpects]

That sounds like making the entirety of lombok pluggable.

Which it already is, but it's not as easy as you make it sound, as you
have to write the transformator both againt eclipse's AST and javac's
AST, and generalizing this into one API is prohibitively difficult, as
the two are so different.

[GHad]

Replies inline

On 23 Aug., 11:10, Reinier Zwitserloot <reini...@gmail.com> wrote:

> (immutable=true).
>
> In order to operate correctly, @Getter needs to know from an object:
>
> A) Are you already immutable?
>
> B) If you aren't, how do I clone you?
>
> C) Do you have a 'create immutable wrapper' type of feature, or can I
> clone you into an immutable object?
>

> I agree with you that, if we find a solution to how to answer these 3
> questions, the addition of an explicit parameter on @Getter reduces
> the magic factor enough for it to be an acceptable addition to lombok.
>

You are right, I was too short-sighted about the consequences such an
addition would produce.

Sometimes a developer needs a flat clone, sometimes a deep one and
somtimes he wants to have it

exposed, so as long as noone cares about writing custom getters there
is no need to target

this. I would just state this at the docs/examples as a "Be careful"
advice about exposing

Collections from automaticly generated getters.

>
> public @interface Data {
> public @interface NoSetter {}
> public @interface NoGetter {}
>
> }
>
> That seems a fair compromise between sticking too much crud in the
> lombok package and offering this flexibility, which does have its
> place. File a ticket for it?
>

Yes, sure, it's a good compromise. Ticket is on in some minutes.

So we will get this situations with @Data and a field foo:

- I want public setter and getter: No additional annotation neccessary
- I want a protected setter but a public getter:

@Setter(AccessLevel.PROTECTED) private Object foo;

- I want no setter, but a getter:

@NoGetter private Object foo;

- I want no accessors at all:

@NoSetter @NoGetter private Object foo;

Seems acceptable for me, it's just a decent declarative clutter,
because if you are using @Data, you have mostly accessable fields and
in the last case, not too much clutter is produced. But the "I want no
accessors at all" brings me back to my previous two solutions:

- If transient fields would be ignored, no Annotations were needed for
this case
- Changing the behavior of Getter/SEtter creation would not help at
all

So, if you're not convinced by the transient field solution, I'll go
with your proposal too.

>
> > Yes there are multiple advantages for using a ReentrantLock over an
> > object array and synchronized:
>

> I'm thinking we should solve this instead by adding closures via
> lombok. Which is a major project and won't happen overnight.
>
> Sticking the options on lombok's annotations also means it's not
> extensible at all, and this again strikes me as a patchy mess and not
> as an elegant solution.
>

I didn't get the turn from different @Synchronized implementations to
closures, but anyway... Where else should this be declared? I think,
if a developers starts messing with parallelism only offering one
solution with @Synchronized is no good choice. Not all of my proposed
enums must be implemented, I just wanted to show that there are many
interesting options for a rich-featured customizable synchronization
under the hood. If a developer doesn't need this, he must not use. The
default would still be like today. Would you rather let the developer
implement the transformation, if he wants a custom behavior? I don't
think so. He would rather implement the behavior directly or use some
aspect oriented toolage before declaring lombok transformations. I
think Lombok itself is the right place, as it is extensible by every
one who likes to contribute. But the developer who wants to use
@Synchronized in his project just needs reasonable options and a solid
default. That's what Lombok could provide.

>
> > [@Fleunt is] worth it and @Builder could automaticly reuse this functionality.
>
> Probably. We'll tackle this when @Builder is added, which might be a
> while. Again, many of these proposals have plenty of less-than-elegant
> issues, and are often not as compelling as some of the things already
> proposed (such as @Delegate) that are completely elegant, so we'll
> work on those first. We're just 2 guys spending some of our spare time
> on this project!

Yeah and it's a great project and you two are doing good work! So
please just think about splitting @Fluent from @Builder when it's
about time. Thanks.

> > You may consider to remove @NotNull @NonNull from the release again,
> > if JSR303 compatibility is reached?
>

> Of course, if people -choose- to use JSR303, lombok should honour that
> too.

Err, my fault... Didn't read enough to understand how they work.
Sorry, that one is done

> > I think
> > I'll file in an issue [on JSR303] when requirements are clear.
>
> Yes, please do, and thanks.

Will take some time though as I first want to get into code until next
weekend and then I'll see further.

> > Yes, I think a plugable solution would be best to encourage develpers
> > to code their own annotations and transformations. Also other APIs
> > would be able to support Lombok with own annotations and create
> > boilerplate code for the developer.
>
> But lombok is already pluggable. Okay, the @Setter/@Getter annotations
> are not, but before putting in the considerable effort, lets first see
> how many other projects are willing to invest the time. If a few are,
> then clearly making Setter/Getter pluggable should result in lots of
> happy faces and a lot more projects taking the effort. If nobody does,
> well, as you can see, we've got enough issues and ideas on the
> backburner to last us for a _DECADE_, so we need to pick and choose
> what to work on.
>
> > [Transpects]
>
> That sounds like making the entirety of lombok pluggable.
>
> Which it already is, but it's not as easy as you make it sound, as you
> have to write the transformator both againt eclipse's AST and javac's
> AST, and generalizing this into one API is prohibitively difficult, as
> the two are so different.

Why is this a matter of how many other projects invest time? The main
benefit would be for the deveolper who is able to write his own
transformations like he writes his own aspects today. But I see the
problem but do not see enough... When I'm into code finaly, I'll try
to abstract this, okay?

BTW, thanks for the discussion and the time you spent on a crazy guys
feedback ;-)

Greetz,
GHad

[Reinier Zwitserloot]

If you don't want accessors at all, just use @EqualsAndHashCode and
@ToString; you'll have to roll your own constructor, which has not
been split out from @Data (and unless there's a need to parameterize
it, I don't think it's worth the clutter), but that's probably a
better solution than @NoGetter/@NoSetter on every single field.

I jumped from Synchronized to closures because of extensibility:
Lombok is not really supposed to be some sort of arbiter that decrees
from up on high which library solutions (involving locking) are
supported. As lombok is fundamentally a compile-time technology,
making it extensible is just not feasible - at least, not in the way
people are used to things becoming extensible.

The solution, therefore, must also be found in a runtime
transformation. And that's where closures come in. If you had
closures, you could do something like:

public void myMethod() {
lock.run({
// your method body here.
});
}

which seems a lot nicer than a lock.lock() call + a try/finally block
to release the lock. The point of that particular code snippet is
this: It's runtime extensible. You want to roll another variant of
locking? Go right ahead. The code reading complexity of that form, vs.
@Synchronized, is relatively minor. Yes, it's another indent, but
that's where it ends, and you do get the flexibility of picking the
exact start and end point of the scope, whereas @Synchronized can only
be applied to an entire method. That easily weighs against the indent.

The principal purpose of @Synchronized is to 'fix' the synchronized
modifier, which can almost be considered a bug - in that the object
it locks on is almost invariably a really stupid monitor object. It's
not meant to be a comprehensive solution to multi-threaded programming
in java. That would require much more thought; merely tossing
alternative locking strategies at @Synchronized is only a tiny
fraction of that. Besides, locking is not even a pain point - the try/
finally idiom works reasonably well. Here's a real blister of a
painpoint in regards to multi-threaded programming in java: fork/join.
fork/join needs closures so bad, the team behind it spend most of
their time cheerleading for closures. Lombok could be of a _much_
greater service making fork/join a boilerplate-free breeze to use.

I'm not at all opposed to fixing this - synchronized, even done
'properly' on private objects, is still a pretty piss poor excuse for
multi-thread management, but its trivially easy for anybody who
already knows java to understand, it doesn't need to be extensible
because synchronized isn't either. The reason it's in lombok at all is
mostly as a hook for the extremely talented multi-threaded guys to
hopefully get excited about lombok. That, and it was very very easy to
implement.


We care about other projects because the effort we've gone through to
make lombok extensible has two primary functions: (A) It makes the
codebase cleaner, which is always a good thing, and (B) it should
allow others with good ideas to maybe start submitting patches to us.

We never intended lombok's extensibility to be used so people roll
their own little DSL-ish features for the project du jour - things
that would never be applicable anywhere else. But, hey, let a thousand
flowers blossom - if that's what loads of people do, that's great!
But, if lombok is to cater to that market, we should do it right. The
extensibility system is definitely not geared toward average
programming. It's complicated, there are a lot of caveats, and you
really need to be quite familiar with especially the eclipse internals
in order to make a transformation that works appropriately. We'd have
to do a lot more work in generalizing things if lombok is ever going
to take off as DSL engine (or, at least, I think it won't). Hence my
focus on not going too much further with that until there's a likely
payoff - which would have to come in the form of patches.