On Wed, Sep 10, 2008 at 9:59 AM, Brian Eaton <[EMAIL PROTECTED]> wrote:
> Re: removing the fetcher factory. Please wait on this. This is a > scary enough merge as is, I don't want to pick up that integration > effort right now. Let's look at it again in a couple of weeks. I think the current integration effort is even more confusing though, with OAuthFetcher taking the HttpRequest in both the ctor and in the call to fetch. This shouldn't be in both places. It should either be passed to the ctor, with HttpFetcher.fetch being a no-arg method, or the work currently being done in the ctor should be done in the fetch method. I have a slight preference for the latter to avoid having to change the HttpFetcher signature. An inner class could probably work well here. As near as I can tell there's no compelling reason not to have OAuthFetcher and HttpFetcher be singletons, and the code path becomes a lot simpler if they are. The only short term change I'm planning on making is moving the switch logic completely into ContentFetcherFactory, so callers can simply call ContentFetcherFactory.fetch(HttpRequest), and that'll delegate appropriately. This eliminates the redundant code between preloading, proxied content, and makeRequest. > > > Re: doc for additional classes: which ones are confusing? I took a > pass through the code and tried to fill out more detail where I > thought was useful, but apparently I was wrong about where it was > needed. MakeRequestClient is particularly confusing -- as near as I can tell it's only used in test code, but there aren't any comments indicating what it's for (I mistakenly thought it was being used by MakeRequestHandler).
