@lombok.Enum for generating finder methods

8,984 views
Skip to first unread message

Jan

unread,
Dec 22, 2017, 8:02:58 PM12/22/17
to Project Lombok
I have implemented a handler for enhancing enums (https://github.com/rzwitserloot/lombok/pull/1545).

So you could write

@lombok.Enum
enum Colour {
   
    RED(255,0,0,"red","rot"),
    BLUE(0,255,0,"blue","blau"),
    GREEN(0,0,255,"green","grün");
   
    private int red;
    private int green;
    private int blue;
    private String englishName;
    private String germanName;
   
}

instead

@lombok.Enum
enum Colour {
   
    RED(255,0,0,"red","rot"),
    BLUE(0,255,0,"blue","blau"),
    GREEN(0,0,255,"green","grün");
   
    private int red;
    private int green;
    private int blue;
    private String englishName;
    private String germanName;

        private Color(final int red, final int green, final int blue, final String englishName, final String germanName) {
        this.red = red;
        this.green = green;
        this.blue = blue;
        this.englishName = englishName;
        this.germanName = germanName;
    }
   
    public static Color findByRed(final int red) {
        for(Color value : Color.values()) {
            if (value.red == red) {
                return value;
            }
        }
        return null;
    }
   
    // same finder for green and blue
   
    public static Color findByRed(final String englishName) {
        for(Color value : Color.values()) {
            if (value.englishName.equals(englishName)) {
                return value;
            }
        }
        return null;
    }
   
    // same finder for germanName
}


Draft supports handling of existing user written finders (don't overwrite them).

What do you think?

Bart Enkelaar

unread,
Jan 3, 2018, 5:31:47 PM1/3/18
to Project Lombok
Though I like this idea for finders, I would put it on Field level instead of Class level. Being an Enum does not automatically mean that I want to find the specific enum by one of its field values (especially not on all).

Perhaps a @FindBy or @GetBy method annotation.

I'd also enhance it with the option to return an Optional or throw an error if there is no Enum with that particular value present (Perhaps having some kind of FailureHandling argument or something like that).

What if the value you find by is non-unique? Ideally I'd expect a compile error or else you'd have to return a Collection of sorts (LinkedHashSet?).

Regards, Bart

Reinier Zwitserloot

unread,
Mar 19, 2018, 5:36:36 AM3/19/18
to Project Lombok
This looks cool but:

* Let @RequiredArgsConstructor take care of making that constructor; this is fundamentally a per-field kind of deal. (As Bart Enkelaar said).
* Call it @FindBy and not @Enum
* Make it a per-field annotation, not an annotation that goes on the enum.
* Calling .values() is an unfortunate anti-pattern (it is inefficient, and enums are sometimes used in cases where this performance hit is noticable). You need to cache it. (private static final MyEnumType[] VALUES = values();)
* For really large enums (100+) a map can be better... do we have a 'useMap' field, which defaults to 'if <100, don't, otherwise, do?'

Bohemian

unread,
Jun 28, 2019, 2:13:00 AM6/28/19
to Project Lombok
I would like to see this implemented, and I am willing to contribute to the lombok code base to get it done.
Is there any traction on this idea?

Reinier Zwitserloot

unread,
Jul 1, 2019, 5:51:39 PM7/1/19
to Project Lombok
We would accept a PR with this feature.... if it looks anything like what we have in mind. Some concerns:

1. The API is such that it'll search directly (loop over all enum values until a match is found), using ==, for all primitives except double and float.

2. @FindBy on a float or double value is an error (equality for these things is just too hairy to deal with).

3. For non-primitives, .equals() is used, and null is dealt with correctly (not NPE, but == instead). There is either no way to specify alternate equalities (notably, no way to search case insensitively), or you have a _REALLY_ good idea as to how to try to tackle it.

4. the code will make a local (private static final EnumType[] VALUES = values();) copy of the values() invocation. (values() is an expensive method, you don't want to call it all that often). Alternatively prove that hotspot will optimize it.

5. PR includes the implementation for both eclipse as well as javac, AND the docs page + a bunch of tests.

6. As the feature will start in experimental, for now no need to coordinate with the intellij plugin team on adding it there.

7. The behaviour is such that if the result is not found, one of two things happens: Either [1] one of the enum values is explicitly marked as the not found default (with the @FindBy.NotFound annotation), in which case that is returned if no search result is found, or [2] there is no such marked value, in which case null is returned. I can foresee folks wanting an exception instead, but to those I say: Create an 'UNKNOWN' sentinel value and use that via @FindBy.NotFound. I can even foresee folks wanting the method to return Optional<ET>, but we'd veto any PR that includes that functionality; we're not particularly big fans of optional, you see.

If you're having trouble with any of it, by all means, let us know, ask for help, share your branch, etc.

Future concerns:

Maybe one day the feature is expanded and you can ask lombok to implement this via a map-based lookup instead; possibly if you don't explicitly specify which variant you want, lombok looks at the # of enum constants inside the enum, and picks the map mode if it's large, and the loop if it is small, with the boundary value decided by analysis of when the map will outperform the loop. But let's not start making it that complicated.

Small chance that hotspot can eliminate the inherent array copy inside the implementation lf EnumType.values(), which would suggest we should NOT make that copy. In that case, we need some sort of proof (by way of 'this bit of code in hotspot solves precisely this problem', or a JMH report). That research would be an awesome contribution, too :)

Custom search mappings might be a concern. Imagine wanting to do a case insensitive search, or a search by string, but the idea is to take the string, parse it into a localdate, and then search with that. I don't see a particularly clean way to support any of this, but perhaps it can be added on later. If someone can come up with a nice design to do it, by all means, let's hear it:)

Tozaicevas

unread,
Jul 2, 2019, 11:41:31 AM7/2/19
to Project Lombok
I'd love to contribute as well, but I am new to lombok, so it might take some time and I'd probably need some help at the start. Hit me up if you're still interested in this.

Martin Grajcar

unread,
Jul 2, 2019, 7:30:12 PM7/2/19
to project...@googlegroups.com
On Mon, Jul 1, 2019 at 11:51 PM Reinier Zwitserloot <rein...@gmail.com> wrote: 
in which case that is returned if no search result is found, or [2] there is no such marked value, in which case null is returned. I can foresee folks wanting an exception instead, but to those I say: Create an 'UNKNOWN' sentinel value and use that via @FindBy.NotFound.

The UNKNOWN value is a good solution for this problem, unless you need to know what search term lead to the problem. Anyway, this good solution may be a bad idea overall. For example, my enum members appear in a few switches and there's nothing sane I could put in case UNKNOWN. Moreover, I use Hibernate and I surely don't want the sentinel value to get stored by mistake in the DB. Additionally, I send my enum values to the client and the client shouldn't show UNKNOWN in its <select>s. This all is solvable, but it makes the solution ugly.

I've implemented a few findBys manually and I let them all throw an exception, cause otherwise I could end up with null or UNKNOWN and no clue what wasn't found. Sometimes, I needed to avoid the exception, so I added something like findByEnglishNameOrDefault (inspired by Map.getOrDefault). Something like Optional.orElseGet(Supplier<Color>) might be useful, too (though findByEnglishNameOrApply(Function<String, Color>) might be better).
 
I can even foresee folks wanting the method to return Optional<ET>, but we'd veto any PR that includes that functionality; we're not particularly big fans of optional, you see.

Sure!

Maybe one day the feature is expanded and you can ask lombok to implement this via a map-based lookup instead; possibly if you don't explicitly specify which variant you want, lombok looks at the # of enum constants inside the enum, and picks the map mode if it's large, and the loop if it is small, with the boundary value decided by analysis of when the map will outperform the loop. But let's not start making it that complicated.

I'm afraid, the Map loses for 99% enums out there, however, it's maybe simpler than the loop:

private static final ImmutableMap<String, Color> BY_ENGLISH_NAME = Arrays.stream(values()).collect(ImmutableMap.toImmutableMap(e -> e.englishName, e -> e));

This also has the advantage of instantly getting an ExceptionInInitializerError in case of ambiguity.

With primitives, things get (much) more complicated.

Small chance that hotspot can eliminate the inherent array copy inside the implementation lf EnumType.values(), which would suggest we should NOT make that copy. In that case, we need some sort of proof (by way of 'this bit of code in hotspot solves precisely this problem', or a JMH report). That research would be an awesome contribution, too :)

Custom search mappings might be a concern. Imagine wanting to do a case insensitive search, or a search by string, but the idea is to take the string, parse it into a localdate, and then search with that. I don't see a particularly clean way to support any of this, but perhaps it can be added on later. If someone can come up with a nice design to do it, by all means, let's hear it:)

I'm afraid, there's no nice design for this. Even such trivial things like case-insensitivity get too complicated once you leave ASCII (and you can add Unicode normalization and locale-specific collations and more, if you really want to). And for every implemented feature, there'll be many refused feature requests.

Reinier Zwitserloot

unread,
Jul 5, 2019, 11:27:30 AM7/5/19
to Project Lombok
Some good points here, Maaartin.

1. ambiguities. Rats, I hadn't thought of that. I guess that's a caveat emptor kinda deal? If you really want a FindBy on a non unique mapping dimension, I guess well then that's on you. mapping to an immutable map WOULD get you an initializer error but possibly that's not what people want (the loop-and-return-on-result strategy gives you the first matching enum, I can see how someone would want that, though I question somewhat the code quality if that's the plan), and class initializer errors are a bit heavyhanded (possibly that's a good thing, you'll definitely know very quickly during development you've made this mistake). Note that using stream means we're bound to java8. Lombok technically still is compatible with java6. We can spec this feature to just blow up in your face if you attempt it on j7- code, though. Surely its time has come :) - more problematic is ImmutableMap: That's guava. We can't assume that. uhoh. Also it does not allow null which in this case is an annoyance. I don't know if .collect(Collectors.groupingBy(e -> e.englishName)) gives us the 'crashes on conflicts' functionality, .groupingBy's javadoc doesn't appear to give any guidance there.

2. throwing the exception.

I'm more convinced you're on to something there. We could go with: If you have a @FindBy.NotFound marked element, that's returned, and if you do not, an exception is thrown. a return value of null is not possible (We COULD, I guess, make it configurable on the @FindBy annotation itself...., but then I think the default is to go with the NotFound, and if that doesn't exist, the exception. The null return occurs only if you explicitly ask lombok to do it that way).

Martin Grajcar

unread,
Jul 6, 2019, 11:12:03 AM7/6/19
to project...@googlegroups.com
Thanks. I'm not saying that this is the implementation to be used, but it's surprisingly simple, so probably a good starting point.

Concerning J8, I guess, for new features it's fine, even when there'll be some older versions for a few years.

Concerning Guava, it's use could depend on the setting useGuava, but better not as this makes things more complicated. Sticking with a single implementation sounds much better.

Concerning conflicts, Collectors.groupingBy is not the counterpart to ImmutableMap.toImmutableMap. It's Collectors.toMap which uses throwingMerger:
The only disadvantage when compared to Guava of it is that you'd get some Map instead of an ImmutableMap, but as it's private, nobody cares.

I always prefer failing fast, so even an ExceptionInInitializerError is welcome. Even with my lacking tests, such a bug has zero chance of going through. It's deterministic, unless initialized e.g., via system properties, but why should we assume that "solving" a conflict via the first match is any better than simply crashing? Someone who didn't bother to test or to read the docs most probably didn't bother thinking about consequences of conflicts either, so in the end, they may be grateful for the crash.

There might be options for conflict resolution (PREFER_FIRST, PREFER_LAST, ...) implemented via a trivial custom merger, but I'm sceptical about their usefulness.

Sure, with @FindBy.NotFound, it should return the marked element. Returning null, only if explicitly requested, otherwise throwing, yes.

---

As a crazy generalization, we could allow annotating a static method, taking either no arguments or the search term like in

enum Example {
    RED(255, 0, 0, "rot"),
    BLUE(0, 255, 0, "blau"),
    GREEN(0, 0, 255, "grün"),
    ;

    private final int red;
    private final int blue;
    private final int green;
    @FindBy private final String germanName;

    @FindBy.NotFound
    private static Example notFound(String germanName) {
        logger.error("No such Farbe: {}", germanName);
        return RED;
    }
}

I'm not really proposing this. It's very flexible, but possibly not worth the effort as it needs about as much handwritten code as much gets saved. OTOH, for someone using  @FindBy.NotFound on RED, and wanting to add logging, it'd be right thing.




--
You received this message because you are subscribed to the Google Groups "Project Lombok" group.
To unsubscribe from this group and stop receiving emails from it, send an email to project-lombo...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/project-lombok/81f1fa85-7114-4f40-8eab-4a36a1356ce4%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reinier Zwitserloot

unread,
Jul 8, 2019, 3:35:27 PM7/8/19
to Project Lombok
Well hey if you don't want the credit for proposing that, I'll take it :)

This is perfect; that's also a way to get to null:

     @FindBy.NotFound private static EnumType notFound() {return null;}

The method must be static, must return the enumtype, and can optionally take 1 parameter.

One annoyance is how to deal with multiply @FindBy annotations, for example one that's on String and another that's on YourCustomType, and then you have a NotFound on a method whose first param type is CharSequence. Lombok doesn't know if a YourCustomType is a subtype of CharSequence. I guess the rule is: We'll call your method, and pass in the variable. If it doesn't fit, well, a compiler error will ensue. On you to fix that / to make the first param's type 'Object' so everything works (autoboxing if needed).
To unsubscribe from this group and stop receiving emails from it, send an email to project-lombok+unsubscribe@googlegroups.com.

Martin Grajcar

unread,
Jul 9, 2019, 6:23:30 PM7/9/19
to project...@googlegroups.com
On Mon, Jul 8, 2019 at 9:35 PM Reinier Zwitserloot <rein...@gmail.com> wrote:
Well hey if you don't want the credit for proposing that, I'll take it :)

Enjoy it!

This is perfect; that's also a way to get to null:

     @FindBy.NotFound private static EnumType notFound() {return null;}

The method must be static, must return the enumtype, and can optionally take 1 parameter.

One annoyance is how to deal with multiply @FindBy annotations, for example one that's on String and another that's on YourCustomType, and then you have a NotFound on a method whose first param type is CharSequence. Lombok doesn't know if a YourCustomType is a subtype of CharSequence. I guess the rule is: We'll call your method, and pass in the variable. If it doesn't fit, well, a compiler error will ensue. On you to fix that / to make the first param's type 'Object' so everything works (autoboxing if needed).

What about overloading?
@FindBy.NotFound private static EnumType notFound(int needle) {return RED;}
@FindBy.NotFound private static EnumType notFound(String needle) {return BLUE;}
@FindBy.NotFound private static EnumType notFound(Object needle) {return GREEN;}
I don't think it gets used much, but it'd surprising if it didn't work. I hope, Lombok just calls the method and the usual overload rules apply.

This allows to define specific defaults for different methods as long as their arguments differs by type. Surely not perfect, but it's free.

Jan Matèrne (GMail)

unread,
Jul 11, 2019, 5:55:08 AM7/11/19
to project...@googlegroups.com

I try to sum up:

 

@FindBy

Generates a finder method for on enum using the annotated field as search parameter.

 

    Lombok

    ------------------------------------------------

    @RequiredArgsConstructor

    public enum Colour {

        RED(255, 0, 0, "red"),

        BLUE(0, 255, 0, "blue"),

        GREEN(0, 0, 255, "green")

        int red, blue, green;

        @FindBy

        String name;

    }

    ================================================

    VanillaJava

    ------------------------------------------------

    public enum Colour {

        RED(255, 0, 0, "red"),

        BLUE(0, 255, 0, "blue"),

        GREEN(0, 0, 255, "green")

        int red, blue, green;

        String name;

        // private constructor generated by @RequiredArgsConstructor

       

        // Generated by @FindBy

        public static ??? findByName(String search) {

            // search for enums with field 'name' is the same as parameter 'search'

        }

    }

    ------------------------------------------------

 

 

Discussion topics

===============================

 

How to compare the field with the search parameter?

---------------------------------------------------

Compare two objects with equals(): enumfield.equals(search)

Compare two primitives with equal: enumfield==search

Compare two floating values with equal and delta: Math.abs(enumfield-search) <= delta

    What delta should be used?

    Or should the search by floating values be forbidden (= compile error)

 

 

What type of return value should be used?

-----------------------------------------

When using a search we could find 0, 1 or multiple elements.

We have

    InstanceType: for returning 0 or 1 element             XXX

    Optional<T> : for returning 0 or 1 element

    List<T>     : for returning any number of elements

 

 

How to specify that multiple elements could be found?

-----------------------------------------------------

We have to return a List<T>. But how tell Lombok to use a List<T> instead of T?

We could specify that on the annotation: @FindBy(LIST|SINGLE)

 

 

How should the finder behave if nothing is found?

-------------------------------------------------

A: return null                                                       

B: throw an Exception (which one?)

C: specify a default value by annotating a constant with @FindBy.Default

D: specify a default method (private static parameterless) which returns the default

  D1: specify the method name in the @FindBy annotation: @FindBy(defaultMethod="nameDefault")

  D2: specify the method name by name pattern: @FindBy name -> findByName() -> defaultName()       

 

 

How should the finder behave if multiple elements found but only one returns?

-----------------------------------------------------------------------------

If the method can return only 1 element (T or Optional<T>) but found multiple elements, this

can be because

a) the search field is not unique

b) there is an error in the enum data

      BLUE(0,0,255,"blue"),

      LIGHTBLUE(0,0,128,"blue") // should be "lightblue"

 

The finder could

- return just the first hit or

- throw an exception (which one)

- maybe create a compiler error?              

 

 

Performance hints

-----------------

- Cache the return of values() in a private static array instead of using values() in each generated finder.

 

 

Future concerns

---------------

- Maybe one day the feature is expanded and you can ask lombok to implement this via a map-based lookup instead;

  possibly if you don't explicitly specify which variant you want, lombok looks at the # of enum constants inside the enum,

  and picks the map mode if it's large, and the loop if it is small, with the boundary value decided by analysis of when

  the map will outperform the loop. But let's not start making it that complicated.

- Small chance that hotspot can eliminate the inherent array copy inside the implementation of EnumType.values(),

  which would suggest we should NOT make that copy. In that case, we need some sort of proof

  (by way of 'this bit of code in hotspot solves precisely this problem', or a JMH report).

  That research would be an awesome contribution, too :)

- Custom search mappings might be a concern. Imagine wanting to do a case insensitive search, or a search by string,

  but the idea is to take the string, parse it into a localdate, and then search with that.

  I don't see a particularly clean way to support any of this, but perhaps it can be added on later.

  If someone can come up with a nice design to do it, by all means, let's hear it:)

 

 

"Management Requirement"

------------------------

1. The API is such that it'll search directly (loop over all enum values until a match is found),

   using ==, for all primitives except double and float.

2. @FindBy on a float or double value is an error

   (equality for these things is just too hairy to deal with).

3. For non-primitives, .equals() is used, and null is dealt with correctly (not NPE, but == instead).

   There is either no way to specify alternate equalities (notably, no way to search case insensitively),

   or you have a _REALLY_ good idea as to how to try to tackle it.

4. the code will make a local (private static final EnumType[] VALUES = values();) copy of the values() invocation.

   (values() is an expensive method, you don't want to call it all that often). Alternatively prove that hotspot will optimize it.

5. PR includes the implementation for both eclipse as well as javac, AND the docs page + a bunch of tests.

6. As the feature will start in experimental, for now no need to coordinate with the intellij plugin team on adding it there.

7. The behaviour is such that if the result is not found, one of two things happens:

   Either [1] one of the enum values is explicitly marked as the not found default (with the @FindBy.NotFound annotation),

     in which case that is returned if no search result is found, or

   [2] there is no such marked value, in which case null is returned.

   I can foresee folks wanting an exception instead, but to those I say: Create an 'UNKNOWN' sentinel value and use that via @FindBy.NotFound.

   I can even foresee folks wanting the method to return Optional<ET>, but we'd veto any PR that includes that functionality;

   we're not particularly big fans of optional, you see.

 

 

 

Proposal "keep it easy"

=================================================

* Comparison: dont allow @FindBy on floating values.

  Create a compiler error.

  User could still write their own finder methods directly in the enum.

* Don't generate code if there is the method already exists.

* You could specify the return type as SINGLE or LIST

  @FindBy(SINGLE): (default) return in instance of the enum type: T

    If nothing is found, null is returned.

    Users could wrap that into an Optional by their own:

      Optional.ofNullable(Colour.findByName("unknown")).orElse(RED)

  @FindBy(LIST): return a list of instances of the enum type: List<T>

    If nothing is found, the list is just empty.

* If the return type is SINGLE and there are multiple hits throw a runtime exception.

  Better would be to create a compiler error instead. Is this possible?

* Support default return values by providing a method by name pattern.

  If a method default{FieldName} exists use its return value:

     @FindBy(LIST)  : private static List<T> defaultForName() { ... }

     @FindBy(SINGLE): private static T defaultForName() { ... }

 

Step 1: Agree on Proposal

Step 2: Specify examples

Step 3: 'Translate' examples to tests

Step 4: Implement for one compiler

Step 5: Discuss ;)

Step 6: Implement for other compiler

Step 7: Write doc

 

 

 

    Lombok

    ------------------------------------------------

    @RequiredArgsConstructor

    public enum Colour {

        RED(255, 0, 0, "red", "rot", false),

        BLUE(0, 255, 0, "blue", "blau", true),

        GREEN(0, 0, 255, "green", "grün", false)

        int red, blue, green;

        @FindBy

        String englishName;

        @FindBy

        String germanName;

        @FindBy(LIST)

        boolean safeForRedGreenColourBlindness;

 

        private static Colour defaultForGermanName() {

            return RED;

        }

    }

    ================================================

    VanillaJava

    ------------------------------------------------

    public enum Colour {

        RED(255, 0, 0, "red", "rot", false),

        BLUE(0, 255, 0, "blue", "blau", true),

        GREEN(0, 0, 255, "green", "grün", false)

        int red, blue, green;

        String englishName;

        String germanName;

        boolean safeForRedGreenColourBlindness;

 

        private static Colour defaultForGermanName() {

            return RED;

        }

 

        // Generated by @RequiredArgsConstructor

 

        private Colour(int red, int blue, int green, String englishName, String germanName, boolean safeForRedGreenColourBlindness) {

            this.red = red;

            this.blue = blue;

            this.green = green;

            this.englishName = englishName;

            this.germanName = germanName;

            this.safeForRedGreenColourBlindness = safeForRedGreenColourBlindness;

        }

 

        // Generated by @FindBy

 

        private static Colour[] VALUES = values();

 

        public static Colour findByEnglishName(String search) {

            List<Colour> found = new ArrayList<>();

            for(Colour actual : VALUES) {

                if (actual.englishName == null) {

                    if (search == null) {

                        found.add(actual);

                    }

                } else {

                    if (actual.englishName.equals(search)) {

                        found.add(actual);

                    }

                }

            }

            if (found.isEmpty) {

                return defaultForEnglishName();

            }

            if (found.size()==1) {

                return found.get(0);

            }

            throw new RuntimeException();

        }

 

        private static Colour defaultForEnglishName() {

            return null;

        }

 

        public static Colour findByGermanName(String search) {

            // same as in findByEnglishName()

            // ...

                if (actual.germanName == null) {

                    // ...

                } else {

                    if (actual.germanName.equals(search)) {

                    // ...

            if (found.isEmpty) {

                return defaultForGermanName();

            }

            // ...

        }

 

        public static List<Colour> findBySafeForRedGreenColourBlindness(boolean search) {

            List<Colour> found = new ArrayList<>();

            for(Colour actual : VALUES) {

                // no null check required for primitives

                if (actual.safeForRedGreenColourBlindness == search) {

                    found.add(actual);

                }

            }

            return found;

            // no defaults for lists

        }

    }

    ------------------------------------------------

 

 

What do you think?

 

Jan

Jan Matèrne (GMail)

unread,
Jul 11, 2019, 5:55:10 AM7/11/19
to project...@googlegroups.com

- Maybe one day the feature is expanded and you can ask lombok to implement this via a map-based lookup instead;

  possibly if you don't explicitly specify which variant you want, lombok looks at the # of enum constants inside the enum,

  and picks the map mode if it's large, and the loop if it is small, with the boundary value decided by analysis of when

  the map will outperform the loop. But let's not start making it that complicated.

- Small chance that hotspot can eliminate the inherent array copy inside the implementation of EnumType.values(),

  which would suggest we should NOT make that copy. In that case, we need some sort of proof

  (by way of 'this bit of code in hotspot solves precisely this problem', or a JMH report).

  That research would be an awesome contribution, too :)

- Custom search mappings might be a concern. Imagine wanting to do a case insensitive search, or a search by string,

  but the idea is to take the string, parse it into a localdate, and then search with that.

  I don't see a particularly clean way to support any of this, but perhaps it can be added on later.

  If someone can come up with a nice design to do it, by all means, let's hear it:)

 

 

"Management Requirement"

------------------------

1. The API is such that it'll search directly (loop over all enum values until a match is found),

   using ==, for all primitives except double and float.

2. @FindBy on a float or double value is an error

   (equality for these things is just too hairy to deal with).

3. For non-primitives, .equals() is used, and null is dealt with correctly (not NPE, but == instead).

   There is either no way to specify alternate equalities (notably, no way to search case insensitively),

   or you have a _REALLY_ good idea as to how to try to tackle it.

4. the code will make a local (private static final EnumType[] VALUES = values();) copy of the values() invocation.

   (values() is an expensive method, you don't want to call it all that often). Alternatively prove that hotspot will optimize it.

5. PR includes the implementation for both eclipse as well as javac, AND the docs page + a bunch of tests.

6. As the feature will start in experimental, for now no need to coordinate with the intellij plugin team on adding it there.

7. The behaviour is such that if the result is not found, one of two things happens:

   Either [1] one of the enum values is explicitly marked as the not found default (with the @FindBy.NotFound annotation),

     in which case that is returned if no search result is found, or

   [2] there is no such marked value, in which case null is returned.

   I can foresee folks wanting an exception instead, but to those I say: Create an 'UNKNOWN' sentinel value and use that via @FindBy.NotFound.

   I can even foresee folks wanting the method to return Optional<ET>, but we'd veto any PR that includes that functionality;

   we're not particularly big fans of optional, you see.

 

 

 

Proposal "keep it easy"

            this.englishName = englishName;

            this.germanName = germanName;

            this.safeForRedGreenColourBlindness = safeForRedGreenColourBlindness;

Jan Matèrne (GMail)

unread,
Jul 11, 2019, 5:56:44 AM7/11/19
to project...@googlegroups.com

- Maybe one day the feature is expanded and you can ask lombok to implement this via a map-based lookup instead;

  possibly if you don't explicitly specify which variant you want, lombok looks at the # of enum constants inside the enum,

  and picks the map mode if it's large, and the loop if it is small, with the boundary value decided by analysis of when

  the map will outperform the loop. But let's not start making it that complicated.

- Small chance that hotspot can eliminate the inherent array copy inside the implementation of EnumType.values(),

  which would suggest we should NOT make that copy. In that case, we need some sort of proof

  (by way of 'this bit of code in hotspot solves precisely this problem', or a JMH report).

  That research would be an awesome contribution, too :)

- Custom search mappings might be a concern. Imagine wanting to do a case insensitive search, or a search by string,

  but the idea is to take the string, parse it into a localdate, and then search with that.

  I don't see a particularly clean way to support any of this, but perhaps it can be added on later.

  If someone can come up with a nice design to do it, by all means, let's hear it:)

 

 

"Management Requirement"

------------------------

1. The API is such that it'll search directly (loop over all enum values until a match is found),

   using ==, for all primitives except double and float.

2. @FindBy on a float or double value is an error

   (equality for these things is just too hairy to deal with).

3. For non-primitives, .equals() is used, and null is dealt with correctly (not NPE, but == instead).

   There is either no way to specify alternate equalities (notably, no way to search case insensitively),

   or you have a _REALLY_ good idea as to how to try to tackle it.

4. the code will make a local (private static final EnumType[] VALUES = values();) copy of the values() invocation.

   (values() is an expensive method, you don't want to call it all that often). Alternatively prove that hotspot will optimize it.

5. PR includes the implementation for both eclipse as well as javac, AND the docs page + a bunch of tests.

6. As the feature will start in experimental, for now no need to coordinate with the intellij plugin team on adding it there.

7. The behaviour is such that if the result is not found, one of two things happens:

   Either [1] one of the enum values is explicitly marked as the not found default (with the @FindBy.NotFound annotation),

     in which case that is returned if no search result is found, or

   [2] there is no such marked value, in which case null is returned.

   I can foresee folks wanting an exception instead, but to those I say: Create an 'UNKNOWN' sentinel value and use that via @FindBy.NotFound.

   I can even foresee folks wanting the method to return Optional<ET>, but we'd veto any PR that includes that functionality;

   we're not particularly big fans of optional, you see.

 

 

 

Proposal "keep it easy"

            this.englishName = englishName;

            this.germanName = germanName;

            this.safeForRedGreenColourBlindness = safeForRedGreenColourBlindness;

Reply all
Reply to author
Forward
0 new messages