Re: [Zope-dev] [Checkins] SVN: zope.intid/trunk/ - Remove a dependency on ``zope.container.contained.Contained``
Am 15.05.2009 um 05:32 schrieb Chris McDonough:
Log message for revision 99961: - Remove a dependency on ``zope.container.contained.Contained`` (this is a dumb base class that defines __parent__ and __name__ as None and declares that the class implements IContained).
What's the reason behind this refactoring? Instead of zope.container.contained.Contained the IntIds class now depends on zope.container.interface.IContained plus it redefines the things which are already defined in zope.container.contained.Contained. I do not see why this is better than using the base class.
Changed: U zope.intid/trunk/CHANGES.txt U zope.intid/trunk/src/zope/intid/__init__.py [...]
Modified: zope.intid/trunk/src/zope/intid/__init__.py ===================================================== --- zope.intid/trunk/src/zope/intid/__init__.py 2009-05-15 02:41:49 UTC (rev 99960) +++ zope.intid/trunk/src/zope/intid/__init__.py 2009-05-15 03:31:59 UTC (rev 99961) @@ -24,10 +24,9 @@
import BTrees from persistent import Persistent -from ZODB.interfaces import IConnection from zope.component import adapter, getAllUtilitiesRegisteredFor, subscribers from zope.container.interfaces import IObjectAddedEvent, IObjectRemovedEvent -from zope.container.contained import Contained +from zope.container.interfaces import IContained from zope.event import notify from zope.interface import implements from zope.keyreference.interfaces import IKeyReference, NotYet @@ -37,14 +36,16 @@ from zope.intid.interfaces import IIntIds, IIntIdEvent from zope.intid.interfaces import IntIdAddedEvent, IntIdRemovedEvent
-class IntIds(Persistent, Contained): +class IntIds(Persistent): """This utility provides a two way mapping between objects and integer ids.
IKeyReferences to objects are stored in the indexes. """ - implements(IIntIds) + implements(IIntIds, IContained)
+ __parent__ = __name__ = None + _v_nextid = None
Yours sincerely, -- Michael Howitz · mh@gocept.com · software developer gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 8 · fax +49 345 1229889 1 Zope and Plone consulting and development
On 5/15/09 2:46 AM, Michael Howitz wrote:
Am 15.05.2009 um 05:32 schrieb Chris McDonough:
Log message for revision 99961: - Remove a dependency on ``zope.container.contained.Contained`` (this is a dumb base class that defines __parent__ and __name__ as None and declares that the class implements IContained).
What's the reason behind this refactoring? Instead of zope.container.contained.Contained the IntIds class now depends on zope.container.interface.IContained plus it redefines the things which are already defined in zope.container.contained.Contained. I do not see why this is better than using the base class.
It's a partial step towards getting rid of a dependency that zope.intid has on zope.container. I'm thinking that maybe that IContained interface belongs in some other package (e.g. maybe zope.contained). That Container base class is.. uh.. not complicated, so even if we never do get rid of the zope.container dependency completely, it really doesn't harm anything to not use Contained. Unless you have some nostalgia for it. ;-) - C
On Fri, May 15, 2009 at 2:57 AM, Chris McDonough <chrism@plope.com> wrote:
It's a partial step towards getting rid of a dependency that zope.intid has on zope.container. I'm thinking that maybe that IContained interface belongs in some other package (e.g. maybe zope.contained). That Container base class is.. uh.. not complicated, so even if we never do get rid of the zope.container dependency completely, it really doesn't harm anything to not use Contained. Unless you have some nostalgia for it. ;-)
At the rate we're going, every class and every interface is going to be in a separate package. Keeping the dependency graph clean is great, and there's plenty to do there. But there's also something to be said about being able to keep a substantial portion of it in your head. The cleanliness of the graph isn't so important if most users still can't understand just because there are so many pieces that they wouldn't normally use directly. -Fred -- Fred L. Drake, Jr. <fdrake at gmail.com> "Chaos is the score upon which reality is written." --Henry Miller
Fred Drake wrote:
On Fri, May 15, 2009 at 2:57 AM, Chris McDonough <chrism@plope.com> wrote:
It's a partial step towards getting rid of a dependency that zope.intid has on zope.container. I'm thinking that maybe that IContained interface belongs in some other package (e.g. maybe zope.contained). That Container base class is.. uh.. not complicated, so even if we never do get rid of the zope.container dependency completely, it really doesn't harm anything to not use Contained. Unless you have some nostalgia for it. ;-)
At the rate we're going, every class and every interface is going to be in a separate package.
I don't think we have many examples of this. We have zope.browser as a repository of some interfaces. The other such package off the top of my head is zope.broken, and that one really isn't pulling its weight so I hope the IBroken interface can eventually move elsewhere. (someone, analysis please? perhaps we need a package to hold content-specific interfaces, equivalent to zope.browser. IBroken and IContained might move there.. but see below) There hasn't been a lot of splitting off of classes into new packages as far as I know. We moved a lot from zope.app into zope. to leave the ZMI behind, but I don't think that's been a bad exercise at all.
Keeping the dependency graph clean is great, and there's plenty to do there. But there's also something to be said about being able to keep a substantial portion of it in your head.
Those two goals are not mutually exclusive and in fact mutually supportive. It's not like it was easy to keep a portion in your head before this work started anyway - it was a horrible, horrible, horrible mess. http://faassen.n--tree.net/blog/view/weblog/2009/01/29/0 I'd argue it's easier now that we can actually *read* the graphs. :)
The cleanliness of the graph isn't so important if most users still can't understand just because there are so many pieces that they wouldn't normally use directly.
I appreciate that point. We shouldn't be generating a lot of small pieces if we can help it. That's why it is important as a first impulse to look at existing packages to move an interface into. Sometimes a dependency inversion is the right way forward. I think in the case of IContained it wouldn't hurt us much moving this interface to zope.location, as IContained is just a marker. It's not as clean as we'd like, but the dependency graph will be much cleaner if we do and it's not horrible either. We want both less packages and cleaner dependencies. I don't think we're moving in the wrong direction at present though, so I don't think it's the time to pull on the package-generation breaks just yet. Regards, Martijn
On Friday 15 May 2009, Fred Drake wrote:
Keeping the dependency graph clean is great, and there's plenty to do there. But there's also something to be said about being able to keep a substantial portion of it in your head. The cleanliness of the graph isn't so important if most users still can't understand just because there are so many pieces that they wouldn't normally use directly.
Yes, I have voiced that concern several times myself. I personally do not even understand where all this fear of installing many packages comes from. I think it is because the ZCML is pulling in too many default registrations and people are afraid of those, which I understand. But to create new packages just because you do not want to use other parts of it is silly. Regards, Stephan -- Entrepreneur and Software Geek Google me. "Zope Stephan Richter"
Stephan Richter wrote:
On Friday 15 May 2009, Fred Drake wrote:
Keeping the dependency graph clean is great, and there's plenty to do there. But there's also something to be said about being able to keep a substantial portion of it in your head. The cleanliness of the graph isn't so important if most users still can't understand just because there are so many pieces that they wouldn't normally use directly.
Yes, I have voiced that concern several times myself. I personally do not even understand where all this fear of installing many packages comes from. I think it is because the ZCML is pulling in too many default registrations and people are afraid of those, which I understand. But to create new packages just because you do not want to use other parts of it is silly.
That's not silly at all. I'm not afraid of installing many packages for my applications. But for libraries and little frameworks, I don't like having to depend on 70 other packages even though my library isn't using 99.9% of the code in there in any way. Is that silly? We created zope.container because we don't want to use all the code in zope.app.container. zope.app.container contains the ZMI a lot of code we don't want to be there in our apps as we're not intending to use it. Is that silly? The zope.browser package was created to prevent, among other things, z3c.form from pulling in code from zope.formlib, a completely separate form library. But it wasn't a good situation. People actually got confused and thought they had something to do with each other, in that z3c.form uses zope.formlib, which it doesn't. Is preventing that situation silly? The principle has to do more with *less code* than *less packages*. We're trying to make it possible to use and be aware of less code when considering individual chunks of the code base. To create loose coupling so that you don't have to worry about all chunks of the codebase even though you're only concerned with a little bit of it. To get rid of the code that isn't used a lot (such as the ZMI). If we can make our zope.* packages not rely on a huge amount of other packages that they aren't really using anyway, we stand a larger change understanding them ourselves, and we stand a larger change others might understand them too and adopt them. I could understand these concerns with package creation better if people would point out how the total amount of packages installed for an application or library is increasing (once the applications and libraries have been adjusted to import from the new locations). But I think the amount of packages is decreasing... Regards, Martijn
On 5/15/09 6:27 AM, Fred Drake wrote:
On Fri, May 15, 2009 at 2:57 AM, Chris McDonough<chrism@plope.com> wrote:
It's a partial step towards getting rid of a dependency that zope.intid has on zope.container. I'm thinking that maybe that IContained interface belongs in some other package (e.g. maybe zope.contained). That Container base class is.. uh.. not complicated, so even if we never do get rid of the zope.container dependency completely, it really doesn't harm anything to not use Contained. Unless you have some nostalgia for it. ;-)
At the rate we're going, every class and every interface is going to be in a separate package.
Keeping the dependency graph clean is great, and there's plenty to do there. But there's also something to be said about being able to keep a substantial portion of it in your head. The cleanliness of the graph isn't so important if most users still can't understand just because there are so many pieces that they wouldn't normally use directly.
This particular changes is a nobrainer. It replaces a base class that defines __parent__ and __name__ as None. It just can't be more understandable to have IntIds subclass from Contained for a new developer. When it subclasses from Contained, they have go look over there to see if it does anything interesting and it just doesn't. Breaking small import dependencies like this one is useful even if you don't manage to break any full package dependencies, if only to get the "little stuff" out of the way to start thinking about whether it makes sense to do anything larger. I *didn't* make any other changes because I don't know what the right thing is wrt interfaces and such, and I don't want to make things even worse. - C
Chris McDonough wrote:
On 5/15/09 2:46 AM, Michael Howitz wrote:
Am 15.05.2009 um 05:32 schrieb Chris McDonough:
Log message for revision 99961: - Remove a dependency on ``zope.container.contained.Contained`` (this is a dumb base class that defines __parent__ and __name__ as None and declares that the class implements IContained). What's the reason behind this refactoring? Instead of zope.container.contained.Contained the IntIds class now depends on zope.container.interface.IContained plus it redefines the things which are already defined in zope.container.contained.Contained. I do not see why this is better than using the base class.
It's a partial step towards getting rid of a dependency that zope.intid has on zope.container. I'm thinking that maybe that IContained interface belongs in some other package (e.g. maybe zope.contained). That Container base class is.. uh.. not complicated, so even if we never do get rid of the zope.container dependency completely, it really doesn't harm anything to not use Contained. Unless you have some nostalgia for it. ;-)
Agreed with Chris; IContained interface is very minor and it's easy enough to reimplement. If that helps us reduce dependencies I say let's not worry about this step. We might consider moving IContained to zope.location - it just subclasses from ILocation after all and doesn't add anything besides being a marker interface. zope.intid already depends on zope.location. The Contained implementation could even move to zope.location, actually. Hm, I wonder what code actually *depends* on IContained (instead implements it). Thanks Michael for watching the checkins carefully. Do keep bringing up whatever issue you see here and we'll discuss it. Even if the checkin stands, it can lead to useful discussions nonetheless. Regards, Martijn
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Martijn Faassen wrote:
We might consider moving IContained to zope.location - it just subclasses from ILocation after all and doesn't add anything besides being a marker interface. zope.intid already depends on zope.location. The Contained implementation could even move to zope.location, actually.
Hm, I wonder what code actually *depends* on IContained (instead implements it).
+1 to moving IContained into zope.location. +lots to removing it altogether, if possible: I'm afraid that there are going to be a few intractable component registrations against it "in the wild", however. Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFKDZ/Y+gerLs4ltQ4RAnnEAKCaXlLL1wsA1RUJeUTWwj98DNzeqQCdH2Av uSwUKR0xsNw7H411LagHneY= =WbeS -----END PGP SIGNATURE-----
participants (6)
-
Chris McDonough -
Fred Drake -
Martijn Faassen -
Michael Howitz -
Stephan Richter -
Tres Seaver