I’m more in favor of this than what you had proposed. To be honest, with what you proposed I don’t see much value left in keeping the Level enum.
Yes, I prefer using the enum (obviously, or I wouldn’t have implemented it that way), but if the consensus is to change it to a class, so be it. As for comments on the below, I think it needs a bit of tweaking (I wouldn’t use Hashtable) but it may be a better option than adding more levels. Or we could add the extra levels but not add new methods for them to AbstractLogger. Ralph On Jan 23, 2014, at 7:49 AM, Paul Benedict <pbened...@apache.org> wrote: > It's a neat idea but it's not a Java enum. I think one of Ralph's goal was to > allow client code to use enums. I still think we should continue that path. > At any rate, this will hopefully lead to a synthesis of ideas and agreement. > > > On Thu, Jan 23, 2014 at 8:22 AM, Matt Sicker <boa...@gmail.com> wrote: > Neat idea. I'd update it for proper concurrency, though. I could write a mock > version of this to show what I mean. > > Matt Sicker > > > On Jan 23, 2014, at 2:42, Nick Williams <nicho...@nicholaswilliams.net> > > wrote: > > > > Okay, I finally got a minute to read all of these emails, and... > > > > EVERYBODY FREEZE! > > > > What if I could get you an extensible enum that required no interface > > changes and no binary-incompatible changes at all? Sound too good to be > > true? I proposed this months ago (LOG4J2-41) and it got shot down multiple > > times, but as of now I've heard THREE people say "extensible enum" in this > > thread, so here it is, an extensible enum: > > > > public abstract class Level implements Comparable<Level>, Serializable { > > public static final Level OFF; > > public static final Level FATAL; > > public static final Level ERROR; > > public static final Level WARN; > > public static final Level INFO; > > public static final Level DEBUG; > > public static final Level TRACE; > > public static final Level ALL; > > > > > > private static final long serialVersionUID = 0L; > > private static final Hashtable<String, Level> map; > > private static final TreeMap<Integer, Level> values; > > private static final Object constructorLock; > > > > > > static { > > // static variables must be constructed in certain order > > constructorLock = new Object(); > > map = new Hashtable<String, Level>(); > > values = new TreeMap<Integer, Level>(); > > OFF = new Level("OFF", 0) {}; > > FATAL = new Level("FATAL", 100) {}; > > ERROR = new Level("ERROR", 200) {}; > > WARN = new Level("WARN", 300) {}; > > INFO = new Level("INFO", 400) {}; > > DEBUG = new Level("DEBUG", 500) {}; > > TRACE = new Level("TRACE", 600) {}; > > ALL = new Level("ALL", Integer.MAX_VALUE) {}; > > } > > > > > > private static int ordinals; > > > > > > private final String name; > > private final int intLevel; > > private final int ordinal; > > > > > > protected Level(String name, int intLevel) { > > if(name == null || name.length() == 0) > > throw new IllegalArgumentException("Illegal null Level > > constant"); > > if(intLevel < 0) > > throw new IllegalArgumentException("Illegal Level int less than > > zero."); > > synchronized (Level.constructorLock) { > > if(Level.map.containsKey(name.toUpperCase())) > > throw new IllegalArgumentException("Duplicate Level constant > > [" + name + "]."); > > if(Level.values.containsKey(intLevel)) > > throw new IllegalArgumentException("Duplicate Level int [" + > > intLevel + "]."); > > this.name = name; > > this.intLevel = intLevel; > > this.ordinal = Level.ordinals++; > > Level.map.put(name.toUpperCase(), this); > > Level.values.put(intLevel, this); > > } > > } > > > > > > public int intLevel() { > > return this.intLevel; > > } > > > > > > public boolean isAtLeastAsSpecificAs(final Level level) { > > return this.intLevel <= level.intLevel; > > } > > > > > > public boolean isAtLeastAsSpecificAs(final int level) { > > return this.intLevel <= level; > > } > > > > > > public boolean lessOrEqual(final Level level) { > > return this.intLevel <= level.intLevel; > > } > > > > > > public boolean lessOrEqual(final int level) { > > return this.intLevel <= level; > > } > > > > > > @Override > > @SuppressWarnings("CloneDoesntCallSuperClone") > > public Level clone() throws CloneNotSupportedException { > > throw new CloneNotSupportedException(); > > } > > > > > > @Override > > public int compareTo(Level other) { > > return intLevel < other.intLevel ? -1 : (intLevel > other.intLevel ? > > 1 : 0); > > } > > > > > > @Override > > public boolean equals(Object other) { > > return other instanceof Level && other == this; > > } > > > > > > public Class<Level> getDeclaringClass() { > > return Level.class; > > } > > > > > > @Override > > public int hashCode() { > > return this.name.hashCode(); > > } > > > > > > public String name() { > > return this.name; > > } > > > > > > public int ordinal() { > > return this.ordinal; > > } > > > > > > @Override > > public String toString() { > > return this.name; > > } > > > > > > public static Level toLevel(String name) { > > return Level.toLevel(name, Level.DEBUG); > > } > > > > > > public static Level toLevel(String name, Level defaultLevel) { > > if(name == null) > > return defaultLevel; > > name = name.toUpperCase(); > > if(Level.map.containsKey(name)) > > return Level.map.get(name); > > return defaultLevel; > > } > > > > > > public static Level[] values() { > > return Level.values.values().toArray(new Level[Level.values.size()]); > > } > > > > > > public static Level valueOf(String name) { > > if(name == null) > > throw new IllegalArgumentException("Unknown level constant [" + > > name + "]."); > > name = name.toUpperCase(); > > if(Level.map.containsKey(name)) > > return Level.map.get(name); > > throw new IllegalArgumentException("Unknown level constant [" + name > > + "]."); > > } > > > > > > public static <T extends Enum<T>> T valueOf(Class<T> enumType, String > > name) { > > return Enum.valueOf(enumType, name); > > } > > > > > > // for deserialization > > protected final Object readResolve() throws ObjectStreamException { > > return Level.valueOf(this.name); > > } > > } > > > > Extending it is easy: > > > > public final class ExtendedLevels { > > public static final Level MY_LEVEL = new Level("MY_LEVEL", 250) {}; > > } > > > > I still and have ALWAYS believed this was the best option. If we used this > > option, I would be fine with not adding any new Levels because I could add > > them myself. > > > > Nick > > > >> On Jan 22, 2014, at 7:04 PM, Remko Popma wrote: > >> > >> This is only a problem for webapps, right? > >> Putting log4j jars in WEB-INF/lib avoids that problem (different class > >> loader). > >> Apps that really want to share log4j jars with other apps would need to > >> play nice. Such apps would do well to use a naming convention like Gary > >> suggests. > >> Otherwise, the last to register would overwrite any previous level with > >> the same name. (Should probably emit a StatusLogger warning.) > >> > >> Same intLevel for different names should not be a problem. > >> > >> > >> On Thursday, January 23, 2014, Gary Gregory <garydgreg...@gmail.com> wrote: > >> Playing devils advocate: > >> > >> What happens when different apps register levels with the same name and > >> different intLevels? > >> What happens when different apps register levels with the same intLevel > >> and different names? > >> Should there be a convention that custom level names be FQNs? > >> > >> Gary > >> > >> > >> On Wed, Jan 22, 2014 at 10:05 PM, Paul Benedict <pbened...@apache.org> > >> wrote: > >> As Gary wanted, a new thread.... > >> > >> First, each enum needs an inherit strength. This would be part of the > >> interface. Forgive me if the word "strength" is wrong; but it's the 100, > >> 200, 300, etc. number that triggers the log level. So make sure the > >> interface contains the intLevel() method. > >> > >> Second, we need to know the name, right? The name probably requires a new > >> method since it can't be extracted from the enum anymore. > >> > >> public interface Level { > >> int intLevel(); > >> String name(); > >> } > >> > >> PS: The intStrength() name seems hackish. What about strength() or > >> treshold()? > >> > >> Third, the registration can be done manually by providing a static method > >> (as your did Remko) that the client needs to invoke, or you could have a > >> class-path scanning mechanism. For the latter, you could introduce a new > >> annotation to be placed on the enum class. > >> > >> @CustomLevels > >> public enum MyCustomEnums { > >> } > >> > >> Paul > >> > >> On Wed, Jan 22, 2014 at 8:52 PM, Remko Popma <remko.po...@gmail.com> wrote: > >> Paul, can you give a bit more detail? > >> > >> I tried this: copy the current Level enum to a new enum called "Levels" in > >> the same package (other name would be fine too). Then change Level to an > >> interface (removing the constants and static methods, keeping only the > >> non-static methods). Finally make the Levels enum implement the Level > >> interface. > >> > >> After this, we need to do a find+replace for the references to > >> Level.CONSTANT to Levels.CONSTANT and Level.staticMethod() to > >> Levels.staticMethod(). > >> > >> Finally, the interesting part: how do users add or register their custom > >> levels and how do we enable the Levels.staticLookupMethod(String, Level) > >> to recognize these custom levels? > >> > >> > >> > >> On Thursday, January 23, 2014, Paul Benedict <pbened...@apache.org> wrote: > >> Agreed. This is not an engineering per se, but really more about if the > >> feature set makes sense. > >> > >> Well if you guys ever look into the interface idea, you'll give log4j the > >> feature of getting enums to represent custom levels. That's pretty cool, > >> IMO. I don't know if any other logging framework has that and that would > >> probably get some positive attention. It shouldn't be so hard to do a > >> find+replace on the code that accepts Level and replace it with another > >> name. Yes, there will be some minor refactoring that goes with it, but > >> hard? It shouldn't be. > >> > >> A name I propose for the interface is LevelDefinition. > >> > >> Paul > >> > >> > >> On Wed, Jan 22, 2014 at 6:48 PM, Gary Gregory <garydgreg...@gmail.com> > >> wrote: > >> Hi, I do not see this as an engineering problem but more a feature set > >> definition issue. So while there may be lots of more or less internally > >> complicated ways of solving this with interfaces, makers and whatnots, the > >> built in levels are the most user friendly. > >> > >> I have have lots of buttons, knobs and settings on my sound system that I > >> do not use, just like I do not use all the methods in all the classes in > >> the JRE... > >> > >> Gary > >> > >> > >> > >> > >> -- > >> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org > >> Java Persistence with Hibernate, Second Edition > >> JUnit in Action, Second Edition > >> Spring Batch in Action > >> Blog: http://garygregory.wordpress.com > >> Home: http://garygregory.com/ > >> Tweet! http://twitter.com/GaryGregory > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org > > For additional commands, e-mail: log4j-dev-h...@logging.apache.org > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org > For additional commands, e-mail: log4j-dev-h...@logging.apache.org > > > > > -- > Cheers, > Paul