Adam,
Well, you basically implemented one of the solutions I said I didn't
like earlier, but oh well. And there are a number of places you need to
cast. So the concerns are still valid.
The one question I do have is why does getInstance take in an
ExternalContext? I'm assuming it's because your executing the init
inside of the getInstance object and I'm not sure this is the correct
place for this. My hope in all of this was to be able to explicitly
handle initialization of this object. Plus, nine times out of ten, the
ExternalContext object is ignored in the call to this method.
Either way, don't see as I really have the time to argue. So is it
acceptable to everyone?
Also, there is no
Adam Winer wrote:
Scott,
OK, well, I just went ahead and implemented what I was trying
to say, to see if I'd run into the problems you're describing. I
didn't...
(It's possible I've broken something in portlet land - I only tested
the changes in a servlet environment.)
On 12/21/06, Scott O'Bryan <[EMAIL PROTECTED]> wrote:
Adding getInstance() to the configurator will either force us to cast in
a bunch of different places
No, it doesn't. No casts were required at all, much
less in a bunch of places, and most of the code now
doesn't even have references to the impl class at all.
or to expose the GlobalConfiguratorImpl's
api to the rest of the world (which I don't want to do because they are
applicable ONLY to global configurator. And it won't lock us into an
API we may have to expand later.
As for simply putting the Boolean on the request map, either I'll have
to make a protected constant on the Configurator interface (which has no
bearing on any of the configurators except the global configurator so it
shouldn't go into the ancestor) or I add a porotected (isDisabled)
method (which, again, is only applicable to the GlobalConfiguratorImpl
and therfore shouldn't do into it's Ancestor).
See the code checked in, which does not suffer these probems.
I've never been one to include a feature into an interface or a class
that is applicable in only one instance of that class because it
violated basics OO design principals. The only other option I see here
is to define the constant used to store the boolean in BOTH classes and
hope they remain in sync, but I've never been a big fan of that either.
Again, see the code checked in.
-- Adam
