incubator-adffaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Scott O'Bryan" <darkar...@gmail.com>
Subject Re: [PORTAL] Changes and API review?
Date Wed, 20 Dec 2006 01:17:22 GMT
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


Mime
View raw message