Scott,
You're explaining very well why you want to put this in IMPL.
And why you need a different instance that handles this on
behalf of all other configurators. You're not yet explaining
why you need a whole class to accomplish this, as opposed
to a standard decorator or CoR pattern, etc. I just don't get it.
This one global instance is going to load all other instances,
and invoke all other instances. NO ONE needs to cast to it -
it is all powerful since it is the first (and only) entry point,
and it can decide whether all the others will run or not.
(And "dog slow"? C'mon, you're exaggerating. Hugely.
And describing one potential implementation of one
potential design.)
Yes, I do fight against adding extra code to our
API. For good reason, ya know! Less public API is
good. And, it really worries me when I see a design
that is unlike all the other designs I've seen for this
sort of pattern. I immediately get a gut feeling that
it's not really necessary.
-- Adam
On 12/19/06, Scott O'Bryan <[EMAIL PROTECTED]> wrote:
It's API bloat and I'm also going to have to store some extra privates
on some of these classes as well as expose some additional api's to
support this. I ran into another issue with not implementing the Global
configurator. Take a look at this code.
When used inside of GlobalConfiguratorImpl, the code looks something
like this:
public void disableConfiguratorServices(final ServletRequest request)
{
if(Boolean.TRUE.equals(request.getAttribute(_IN_REQUEST)))
{
throw new IllegalStateException("Request is already begun.");
}
request.setAttribute(_IN_REQUEST, _DISABLED);
}
Now all the IN_REQUEST stuff is basically there to handle proper cleanup
and some API enforcement when calling into the GlobalConfigurator's
getExternalContext method. It is only exposed through the
GlobalConfiguratorImpl. There is no reason for anyone to know about it
outside of the impl package and even then in only a few locations.
Really though, people should keep their hands off of this. If I have on
the normal configurator a getGlobalConfigurator method which returns a
configurator, I have to go though much hokeyness in order to tell
whether I'm in a request or not. Because API cannot depend on classes
in Impl, I'll need to use reflection to get at the private methods
unless I want to expose this as an API on the configurator in general
and I don't think we want to do that.
We can also just skip the validity checking and modify the attribute,
but I would think that if someone is trying to disable the Configurator
and the response has already begun, then they should be notified as soon
as possible before hokey little errors creep up. There is much in the
impl package that depends on the instance of the GlobalConfiguratorImpl
and it's silly in my opinion to have to cast it every time we need to
use an arbitrary function. And when we need functionality from this guy
in the API package (like with the ResourceServlets) the implementation
begins to look REAL ugly.
So my question is why should we have to go through all this reflection
and casting, making this system dog slow, rather then supporting the
proper API's. The amount of work I've had to put in simply to code
"around" these issues is amazing. And I still havn't heard any
compelling reason why THIS implementation is not good.
Scott
Adam Winer wrote:
> Well, in this specific instance, it therefore doesn't "bloat" every
> configurator, since it only appears in one location.
>
> -- Adam
