incubator-adffaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Adam Winer" <awi...@gmail.com>
Subject Re: [PORTAL] Changes and API review?
Date Thu, 21 Dec 2006 05:24:31 GMT
Scott,

What about the far simpler approach of:

  public static final void disableConfiguratorService(ExternalContext external)
  {
    external.getRequestMap().put(..., Boolean.TRUE);
  }

  public static final boolean
isConfiguratorServiceDisabled(ExternalContext external)
  {
    return Boolean.TRUE.equals(external.getRequestMap().get(....));
  }

Eliminates all the introspection and ugly hidden impl dependencies.
*And*, add to Configurator:

  public static Configurator getInstance(ExternalContext)
  {
     ... use code like RequestContextFactory.getFactory(),
     but instead of instantiating a hardcoded file, use
     ClassLoaderUtils.getServices("org.apache.myfaces.trinidad.config.GlobalConfigurator");
     and calling configurator.init(...) to boot
  }

... with, of course,
META-INF/services/org.apache.myfaces.trinidad.config.GlobalConfigurator
pointing
at GlobalConfiguratorImpl.  And put the onus on
GlobalConfiguratorImpl's init() to load (and initialize)
all the other Configurators.

I think this'd also eliminate any need for "isInitialized()" - put
the onus on the code that retrieves (and therefore might
create) the instance.

-- Adam


On 12/19/06, Scott O'Bryan <darkarena@gmail.com> wrote:
> Adam,
>
> EVERYTHING will have to cast to it, since the entry point does not
> return this class, but it's Configurator equivalent.  Reflection and
> casting are among the slowest operations in the Java language.  And, we
> do need access to this from the API package as well as the Impl (unless
> you want me to move the resource servlet to the impl).  These are facts.
>
> Can it be done?  Yes.  But it's really ugly.  So I'll tell you what.
> I'll make an additional patch for this.  Take a look at both and you
> decide which is superior.
>
> Scott
>
> Adam Winer wrote:
> > 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
>
>

Mime
View raw message