Beefy Boxes and Bandwidth Generously Provided by pair Networks
Perl: the Markov chain saw

Re^3: Breaking Tie::Hash into three modules

by afoken (Canon)
on Sep 22, 2018 at 10:32 UTC ( #1222832=note: print w/replies, xml ) Need Help??

in reply to Re^2: Breaking Tie::Hash into three modules
in thread Breaking Tie::Hash into three modules

There is a life outside Perl, that keeps me dry, warm and well-fed. But sometimes, it needs time I wanted to use for perl. So, after an unplanned delay, here are ...

Even more patches


I've downloaded perl 5.28.0 from cpan to a spare machine, unpacked and compiled it, and run its test. No errors, no warnings, no problems. So my testing environment (Slackware 14.2) is ok. I did not expect anything else.

Then, I did the same after changing Tie/, Tie/, Tie/ and moving the Tie::Scalar, Tie::StdArray, Tie::StdHash, and Tie::ExtraHash classes to new files. I've completely removed Carp, warnings::register and new(). The latter is nonsense, as explained in Re: Breaking Tie::Hash into three modules and in Note 1 below. Removing new() made all of the checks and warnings in the dummy TIEARRAY() / TIESCALAR() / TIEHASH() constructors obsolete, and because they contained no useful code beyond that, I just removed them. I also removed dummy methods that just called called croak() together with Carp used only there. If the constructors or required methods are missing, tie will complain as loudly as the code in the dummy constructors and methods did before. No Perl code needed for that.

Perl compiled fine, but unrelated tests failed. Typos, missing edits, you name it. The new files have to be added to MANIFEST. D'oh! Could have thought of that. complains about the new files. Patch that, wash, rinse, repeat.

Strange Tests

With clean code, some tests still failed. Not because the cleanup did not work. It worked fine, not a single test complained about the methods I've moved around or removed. They failed because abusing the classes did not cause the same errors as before.

They failed because the nonsense new() constructor no longer existed that tie never ever calls. lib/Tie/scalar.t explicitly calls Tie::StdScalar->new(). No sane code would do that!

Patching Tests

I've considered a long time if I should treat the tests like a requirement document that must not be changed. And yes, for me, they are a kind of requirement document, but with bad wording. Following the wording would have meant to re-introduce all of that nonsense code around new() just to pass the unmodified tests.

I choose to follow the spirit, but not the words of that implied requirement document. Wrong or missing constructors shall fail, and I won't change that requirement. But I will change the test that checks for the exact error message. And because I've removed the nonsense new(), the tests rotating around that will have to go.

A huge patch

34 kBytes, 1128 lines. Much of the POD for the new files wss copied, and moving stuff from one file to another really inflates diff output. If there was a notation for "move this chunk of text to that file", the output would be much shorter.


(1) Yes, we could make life a little bit easier for people coming from C++, by giving them an additional new constructor faking the C++ new operator. But: tie does not call that constructor, and you don't have a new() constructor in C++ classes. The constructor of a C++ class has the same name as the class. So new() is complete and utter nonsense, even if we want to make Perl look like C++.

(2) As before, the patch is under the same license as Perl itself. Feel free to test it.


Today I will gladly share my knowledge and experience, for there are no sweeter words than "I told you so". ;-)

Log In?

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: note [id://1222832]
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others examining the Monastery: (1)
As of 2022-01-25 23:49 GMT
Find Nodes?
    Voting Booth?
    In 2022, my preferred method to securely store passwords is:

    Results (69 votes). Check out past polls.