Lombok.enum

[janmaterne]

Implemented a draft of a handler for @lombok.Enum, adding a finder method for each field of an enum.
Have checked that manually in Eclipse Oxygen (4.7.0) and Oracle JDK 1.8.

A command line "ant test" went wrong with unrelated errors like "RunTestsViaDelombok cannot be resolved to a type".
Maybe some more docs are required (where?)

Before merging the rebase had to be done (by me) - but I want to discuss before ;-)

[Maaartinus]

Some random comments (I only skimmed over the code):

1. I don't like using names conflicting with common classes. what about @FindBy?
2. I usually need a single lookup, so I'd allow ElementType.FIELD, too.
3. What about findByName(name, defaultValue) or maybe findByName(name, defaultValueSupplier)? Using the supplier would allow throwing, too, which is sometimes preferable.
4. The loop is probably good enough, unless for huge enums, but blindly returning the first match doesn't sound right.

[janmaterne]

Thanks for the comments.
#1: Yeah. Wasn't sure about @enum.
#2: My first idea was having a simple annotation and getting all the stuff. But I could use the annotion for finer tuning: like @getter for the whole enum or just the specified fields. ATM @enum makes a) all fields final, b) adds a finder per field, c) generates a private all arg constructor. When using the annotation on a field, I would generate the constructor too.
#3: could use the defaultValue way; the supplier is also interesting, but requires Java8+. Which Java version should be supported?
#4: Right. Better would be a List. I could finetune that via the annotation: get the first, get all matches; return a List, an Optional or a plain object/null.

[Maaartinus]

1., 2. Agreed

3. Which Java version should be supported?

According to stats I've seen, there are still 30-40% versions older than Java 8 in 2017, so it's a hard question. I'd prefer the supplier version as it's very general and forces you to think about the miss. But with my below proposal, this dilemma vanishes.

4. Better would be a List.

Either this or require uniqueness (which is what I usually do by adding a static ImmutableBiMap field; this is a runtime check which gets executed ASAP, so it's improbable to cause unexpected troubles). As always, there are tons of options with diminishing returns, especially when returning a list.

For the scalar return, there are the following variants:

• SUPPLIER: E findByX(X x, Supplier<E> defaultValueSupplier), Java 8+
• OPTIONAL: Optional<E> tryFindByX(X x), Java 8+, sort of equivalent to the supplier
• DEFAULT: E findByX(X x, E defaultValue), like Map.getOrElse
• SIMPLE: E findByX(X x) defaulting to null

This could be controlled by the value argument like @FindBy(SUPPLIER), etc. The argument should be mandatory unless you choose one in lombok.config. So there's no need to decide if older Java should be supported; the user decides.

Disclaimer: I'm just a random commenter here, so whatever I say, doesn't matter. ;)

I'd await some feedback from the project owners (which may take long 😢).

[rzwitserloot]

@rspilker is going to work on fleshing this out. He has a few modifications in mind.

[mbeko]

In case you don't know about it, you might want to draw inspiration from lombok-pg's @EnumId.

[pbludov]

What about ofX instead of findByX?

 public static MyEnum ofId(int id) ...
 public static MyEnum ofName(String name) ...

This is a well-established name for such methods.

[janmaterne]

The findByX names came from my original thought of "searching the 'database' of existing enum values'.
But you're right, maybe another name would be better.

I read "Effective Java 3rd ed." during my holidays so I quote Joshua Bloch (Item 1, p.9) here for common names:

• from: A type-conversion method that takes a single parameter and returns a correspondig instance, e.g. Date.from(instant)
• of: An aggregation method that takes multiple parameters... e.g. EnumSet.of(...)
• valueOf: A more verbose alternative to from() and of()
• instance or getInstance: returns an instance that is described by its parameters
• create or newInstance: like instance, except that the method guarantees that each call returns a new instance
• get{Type}: like getInstance if used in a different class
• new{Type}: like newInstance if used in a different class
• {type}: a concise alternative to get{Type} and new{Type}, e.g. Collections.list(...)

For the here intented behaviour I think:

• from: possible as it converts an attribute value to the corresponding enum value; the attribute name has to come into the name because there could be multiple of them, e.g. Color.fromName("red"), Color.fromRGB("FF0000")
• of: could think of that, but we have only one parameter, so this would not fit to the criteria above
• valueOf: same as from/of, but I would favour 'from' to this
• instance/getInstance: possible, but not the one I would choose, because passing the attribute name is not easy: Color.getInstanceWithName("red") (very verbose), Map<String,Object> options = "name" => "red"; Color.getInstance(options) (powerful, but easy to use)
• get{Type}, new{Type}, {type}: should be in the enum class itself

So I would prefer the fromBy{AttributeName}(param) over the suggested of{AttributeName}(param).

Generating the options-stuff additionally is an idea ...

@EnumOptionBuilder public enum Color { ... }

which generates something like

public enum Color { public class ColorOptions { // have to think about an easy to use API ;-) } public static Color instance(ColorOptions options) { ... } }

[mbeko]

In the enums which I have written so far and which I would like to have generated by Lombok, I also use from{AttributeName}, so I support this suggestion.

[Frodez]

The foreach iterator's performance is lower than get from a map, especially a immutable map.And somehow a list of value with the same type is also meet the need.
For instance, there is my code.
`

import com.google.common.collect.ImmutableMap;

......

@Getter
private Byte val;

@Getter
private String desc;

@Getter
private static List<Byte> vals;

@Getter
private static List<String> descs;

private static final Map<Byte, Enum> enumMap;

static {
        vals = Arrays.stream(Enum.values()).map(Enum::getVal).collect(Collectors.toUnmodifiableList());
	descs = Arrays.stream(Enum.values()).map((iter) -> {
		return iter.val.toString() + ":" + iter.desc;
	}).collect(Collectors.toUnmodifiableList());
	var builder = ImmutableMap.<Byte, Enum>builder();
	for (Enum iter : Enum.values()) {
		builder.put(iter.val, iter);
	}
	enumMap = builder.build();
}

public static Enum of(Byte value) {
	return enumMap.get(value);
}

`

[vkushni]

Sometime ago i was part of huge project, it was easy to mess all around so they used strict terminology for methods for getting data:
find - when you may get object back and may not and both are valid cases - it should return optional
get - when you most likely expect to get object back - it should return object itself

Since that time i'm following that everywhere and it works to me. My enums look like this, in most cases they have only find method, get methods added rather as example and using 1.6 java may be implemented without optional:

public enum Color
{
    RED,
    GREEN,
    BLUE,

    ; // this one + every constant in new line are dirty tricks to avoid copy&paste issues
// cause usually new constants added by copying old ones
// changing `,` to `;` not a big deal but sometimes it can make you mad.

    public static final Map<String, Color> MAP_BY_NAME = Stream.of(values())
                    .collect(Collectors.toMap(Color::name, Function.identity()));

    public static Optional<Color> find(String name)
    {
        return Optional.ofNullable(name).map(String::toUpperCase).map(MAP_BY_NAME::get);
    }

    public static Color get(String name)
    {
        return get(name, null);
    }

    public static Color get(String name, Color defaultValue)
    {
        return find(name).orElse(defaultValue);
    }
}

[randakar]

As a minor point, from a performance perspective calling values() is slow due to it cloning the internal values array under the covers to guarantee immutability. That's why the static map version outperforms the simple version in the general case. As for naming, Enum should imply getters for any fields*, so I prefer find() for the actual lookup methods. And yes, I prefer them to return Optional**. *) Enums should have fields. Most projects only use the names but that is by far the least interesting use of an enum. And there is almost guaranteed to be a switch or series of if statements somewhere else that could be replaced with a field.. **) Honestly, projects running on 10 year+ old java versions can only get away with that only because projects try to cater to them. At some point it should start to break down for the simple reason that the maintenance burden grows exponentially with the number of supported java versions - staying on java 5 forever is a choice, and doing that is in my view antisocial. Dropping support for those at some point is entirely justifiable.

…

On Sat, Aug 29, 2020, 23:22 Volodymyr ***@***.***> wrote: Sometime ago i was part of huge project, it was easy to mess all around so they used strict terminology for methods for getting data: find - when you may get object back and may not and both are valid cases - it should return optional get - when you most likely expect to get object back - it should return object itself Since that time i'm following that everywhere and it works to me. My enums look like this, in most cases they have only find method, get methods added rather as example and using 1.6 java may be implemented without optional: public enum Color { RED, GREEN, BLLUE, ; public static final Map<String, Color> MAP_BY_NAME = Stream.of(values()) .collect(Collectors.toMap(Color::name, Function.identity())); public static Optional<Color> find(String name) { return Optional.ofNullable(name).map(String::toUpperCase).map(MAP_BY_NAME::get); } public static Color get(String name) { return get(name, null); } public static Color get(String name, Color defaultValue) { return find(name).orElse(defaultValue); } } — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1545 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABIERP5OUKTSPR247IF2S3SDFWPRANCNFSM4EJMBDOQ> .

[yhan219]

Hi, I think this is a very useful feature, do you have a release plan?