I just realized that most of the code in Module/Build/Version.pm is in package version, not Module::Build::Version. I thought an MBV object delegated to a version object and was doing verification checks on it. My mistake.
That eliminates most of my practical objections having to do with _verify() in Module::Build, but I still think its a bad idea in general for version.pm. John Peacock wrote: >> This line of thinking mortgages the future. > > Please remember that the CPAN release of version.pm is just a compatibility > module for the Perl core version object code, which is literally compiled into > the Perl binary. Once v5.10.0 is released, that API is frozen. Any future > CPAN > release has to be compatible with the core code. I have a way to override > what > is in the core (which is what Andreas is going to use), but it happens at > runtime only. Ok. What does this have to do with thinking the internals of version.pm will never change? Or that a subclasser of version.pm might not want to change the internals? Also using an unstable module in the module build toolchain worries me greatly. You can imagine how much it worries me to have it interlocked. >> But you're not testing the methods and behaviors, you're testing the guts. > > At some point in any class, some piece of code is allowed to peer into the > guts. > The constructor gets to know about the internals of the object, because it > is, > you know, *constructing* the object. The public API can use private API > calls, > or even access the object internals directly, because the actual > implementation > is dependent on the actual internal representation. > > Among other things, calling Perl object methods from XS code is slow and > messy. > It would greatly obfuscate the XS code to have all of the public API code > relying on object methods that are not part of the public API to compose or > decompose the object. Ok, but Module::Build::Version isn't XS. Private methods and poking at your own guts are a necessary evil. Just because there are times when they are necessary does not make it a good thing to start doing elsewhere. You want to minimize the amount of mucking around in the object's guts you do, ideally things get set up in an init method (which sets up the guts) and then all the data access goes through public methods. The more private methods you call and guts mucking you do the less flexible your code becomes. The classic example is accessor methods. Where possible your methods should use your own public accessor methods rather than access the underlying hash directly. This allows a subclass to override the accessor behavior and it all Just Works. And the original point remains, _verify() is supposed to be checking that the object retains its original behaviors. But it doesn't. It just looks at the guts. The behaviors can all be changed without touching the guts. >> Either test the behaviors directly via the public API, allowing subclassers >> to >> get creative, or just trust that other people know what they're doing and >> drop >> the check entirely. > > After thinking about it, I realize that this really isn't a matter of > validating > subclasses at all, or even public vs private API; it is a matter of > abstracting > a common bit of code out. I had to do those checks in other parts of code > anyways, which is why I made a private function to do it in one place. Ok, and why do the checks exist in the first place?
