It was a lot harder when each custom extension was an enum implementing an
interface. There was no way to do a new on it.
Ralph
> On Jan 25, 2014, at 10:33 PM, Remko Popma <remko.po...@gmail.com> wrote:
>
> Hm...
> Doesn't it become nearly impossible to protect against malicious intent, the
> moment we make it extensible?
>
>> On Sunday, January 26, 2014, Ralph Goers <ralph.go...@dslextreme.com> wrote:
>> A malicious app could do
>>
>> for (int i=0; i < 100000; ++i) {
>> new Level(“Level” + i, 1000 + i){};
>> }
>>
>> Sure idiots can do lots of bad things but I don’t think Levels should be
>> quite that flexible.
>>
>> Ralph
>>
>>> On Jan 25, 2014, at 9:39 PM, Remko Popma <remko.po...@gmail.com> wrote:
>>>
>>> I don't think client code can do new Level(){} as the constructor requires
>>> String and int arguments.
>>>
>>> By the way, I am unclear on what went wrong with the enum approach you
>>> originally took.
>>> You said:
>>> StdLevel isn’t a Level because it can’t extend it if it is an enum, so
>>> I can’t initialize the levels using that.
>>>
>>> I don't understand. StdLevel implements the Level interface, right? So what
>>> do you mean by "it can't extend it"?
>>>
>>> The reason I ask is that for the code generation, "real" java enums may be
>>> easier to deal with than the extensible enum approach.
>>>
>>>
>>> On Sun, Jan 26, 2014 at 2:30 PM, Ralph Goers <ralph.go...@dslextreme.com>
>>> wrote:
>>> Out of curiosity, what exactly is the benefit of declaring the class
>>> abstract when it has a protected constructor? It seems like all you are
>>> accomplishing is making the instantiation syntax uglier. It also bothers me
>>> that open code can just do a new Level(){} - which will do nothing but
>>> cause problems. I’m beginning to think that we should require an
>>> annotation on the Level declaration of the extension class to avoid that.
>>>
>>> Ralph
>>>
>>>> On Jan 25, 2014, at 9:13 PM, Remko Popma <remko.po...@gmail.com> wrote:
>>>>
>>>> Ralph,
>>>> I copied Nick's code _as is_ and had no compile errors.
>>>> The class is abstract, but instances are defined in the static block as:
>>>> OFF = new Level("OFF", 0) {}; // note the {}: this creates an anonymous
>>>> concrete subclass
>>>>
>>>> I agree that read access needs to be synchronized as well, not just write
>>>> access (the constructor).
>>>> I experimented with several options:
>>>> * synchronizing on plain Object in both constructor and when accessing the
>>>> static Map(s)
>>>> * a ReentrantReadWriteLock
>>>> * a lock-free implementation
>>>>
>>>> I decided against ReentrantReadWriteLock as it has more overhead than
>>>> plain synchronized access and the write access (in the constructors) is
>>>> going to be extremely rare: not worth paying the overhead in the more
>>>> common reads. It is also cumbersome to code.
>>>>
>>>> The lock-free implementation uses an AtomicInteger for the ordinals, and
>>>> an AtomicReference for the Map<String, Level>.
>>>> In the constructor, create a new Map<String, Level> instance based on the
>>>> old copy, add the new instance, and try to call compareAndSet to replace
>>>> the old instance with the new instance. Retry on failure.
>>>>
>>>> Finally, simply synchronizing on the constructorLock object in the
>>>> Level.toLevel() and Level.values() methods may be simplest.
>>>>
>>>> Which of the last two is best depends on how often the toLevel() and
>>>> values() levels are called.
>>>> It turns out they are only called during reconfiguration, so no real need
>>>> to optimize these methods.
>>>> I would argue that simple synchronization may be best in this case.
>>>>
>>>> Remko
>>>>
>>>>
>>>>
>>>> On Sun, Jan 26, 2014 at 1:49 PM, Ralph Goers <ralph.go...@dslextreme.com>
>>>> wrote:
>>>> As I am working on this I just want to point out a number of issues with
>>>> the code below:
>>>>
>>>> 1. The class is abstract. The static block is doing a bunch of new Level()
>>>> invocations which obviously generate compile errors on an abstract class.
>>>> I had to make it be a non-abstract class.
>>>> 2. As I pointed out before there is no way to access the “standard” levels
>>>> as an enum. I have addressed that.
>>>> 3. Although the constructor is synchronized access to the Map is not.
>>>> Trying to get from the map while a Level is being added will result in a
>>>> ConcurrentModificationException. I
