Has it worked so far though? The presence of LoaderSettings seems to be the
source of the headache in all the usages so far, particularly in
testability.

But :+1: for the big paper class discussion idea! Just wanted to get the
ball rolling.
On 12 Jan 2014 10:56, "Paul Blundell" <notifications@github.com> wrote:

> In
> core/src/main/java/com/novoda/imageloader/core/network/UrlNetworkManager.java:
>
> >      private int manualRedirects;
> >
> > +    /**
> > +     * UrlNetworkManager with custom settings.
> > +     *
> > +     * @param connectTimeoutMillis      connect timeout on the URLConnection
> > +     * @param readTimeoutMillis         read timeout on the URLConnection
> > +     * @param disconnectAfterDownload should disconnect after each call
> > +     * @param fileUtil
> > +     */
> > +    public UrlNetworkManager(int connectTimeoutMillis, int readTimeoutMillis, boolean disconnectAfterDownload, FileUtil fileUtil) {
>
> [image: :-1:] mmm but the idea of the Loader settings is for future
> proofing. You could add a new var in LoaderSettings and not have to change
> the public signature of this class.
>
> I'm not sure if you're simplification is just going to more of a
> maintenance headache.
>
> I would have expected you to create another object NetworkSettings that
> encapsulates these 5 fields, you can still have this constructor but it
> should be private and UrlNetworkManager(NetworkSettings should be
> available.
>
> If you want to discuss the ImageLoader I think we need a big piece of
> paper, draw all the classes that are publicly exposed on it and we will
> discuss what is needed and what isn't. Think of the children!
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/novoda/ImageLoader/pull/155/files#r8813392>
> .
>