[prev in list] [next in list] [prev in thread] [next in thread] 

List:       ivy-dev
Subject:    Re: Ivy refactoring - step 3
From:       "Xavier Hanin" <xavier.hanin () gmail ! com>
Date:       2007-02-02 17:22:17
Message-ID: 635a05060702020922v21810f32q6bc8d885c87ffd3d () mail ! gmail ! com
[Download RAW message or body]


On 2/2/07, Stephane Bailliez <sbailliez@gmail.com> wrote:
>
> Xavier Hanin wrote:
> >
> > I'm sorry I must be tired but I don't see how using a factory method
> > instead
> > of a constructor introduces a dependency. Are we really talking about
> the
> > same thing?
>
> It's me not being clear enough.
> I meant the fact that IvySettings is introduced within the factory
> method leads to this.


Ok, now it's clear and I agree. The point is that it's very useful to be
able to create and instance of DeliverOptions from an IvySettings instance,
so I think such a factory/constructor should exist somewhere. But if we want
to avoid the dependency, where should we put this factory?

> I admit that in case of DeliverOptions I don't see any reason why we'd
> > want
> > to subclass it, since it doesn't define any behavior, it's only used to
> > group attributes. So I'm fine with using a constructor instead of a
> > factory
> > method, but I still don't see the advantage.
> >
> > Overall in this specific case dependencies and constraints about the
> >> model are introduced while you get no benefit in return.
> >> Indeed a DeliverOptions is actually created when you deliver...which
> >> means you do it only once in Ivy class.
> >
> >
> > Once again I'm not sure we're really talking about the same thing, but
> > the
> > benefit I see when using a class to store a set of parameters is to
> > have a
> > cleaner and more readable API (methods with less arguments) and more
> > manageable (it's easy to add a new parameter without breaking the
> > API). But
> > I'm pretty sure you agree that methods should have too many arguments,
> as
> > you suggest it's a bad thing for constructors (below). But maybe you
> have
> > another solution than introducing a class like DeliverOptions?
>
> Ah, I see probably where we don't understand each other.
> No I'm not saying that DeliverOptions is a bad idea, just the
> DeliverOptions.newInstance(IvySettings) because of this IvySettings that
> suddenly pops up as a dependency while it could very well not be there.


Ok, got it now!

>
> > Yes, this is a real problem. Another problem is that from the
> > beginning it's
> > possible to specify the directory to use as root for the cache in
> > almost all
> > method calls, instead of always using the default cache configured in
> the
> > settings. So this directory everywhere, instead of using only the
> > settings
> > which are usually available everywhere. A real pain, for something that
> I
> > doubt is even used  :-(
> >
> > Anyway, cache management should really be reviewed and improved, but I
> > don't
> > think it's directly related to the topic of method arguments
> refactoring.
>
> Oh I think it's just a manifestation of it, because cache or something
> else  when you are deep in the chain and suddenly find yourself in a
> position of 'oh s*, I need this information, how can I get it',
> generally we, humans tend to solve that problem as 'I'll give it as an
> argument to...'. Damn humans. :)


+1 But it's sometime difficult to find another way.

Oh and don't misunderstand me I really think what you've done is great
> because that will make it much easier to understand (I'm actually quite
> amazed at the speed you've been doing it), but I'm just no fan in
> introducing lots of methods that can be distracting in an API.


I agree, that's why my intent is to reduce the number of methods in the Ivy
class, to have only one method by feature, removing all those awful
combinations of possible arguments. I understand that in a sense I've moved
some of the combinations to DeliverOptions, but I think that if give only
two possible way to construct a DeliverOptions instance it would be fine:
1) no argument constructor, configure everything using setters
2) use an IvySettings object to configure with sensible defaults

Does it make sense?

Xavier

Not sure I'm much more clear, but I too have very tired eyes and cloudy
> brain. Sorry about that.
>
> -- stephane
>


[prev in list] [next in list] [prev in thread] [next in thread]