Beefy Boxes and Bandwidth Generously Provided by pair Networks
"be consistent"
 
PerlMonks  

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

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


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

Perhaps the way to go would be making the three modules into just one: Tie::Hash. Same with Tie::Array and Tie::Scalar.

And for completeness, also merge Tie::StdHandle into Tie::Handle.

I really like that idea. I shortly thought about it, but I did not have the courage to drastically change the behaviour of Tie::Hash, Tie::Array, Tie::Scalar, and Tie::Handle.

But:

Looking again at the source, there is a pattern in Tie::Array, Tie::Hash, and Tie::Handle, hidden between the nonsense code: All three classes implement methods for inheriting classes that do NOT depend on the actual implementation of the class, but use existing primitive methods. And I think that for this reason, we should keep the class names and functionality as they are now. The generic class implements generic methods that allow omitting some methods in inheriting classes, even with completely different inner structures, and the Std* class implements standard behaviour based on a trivial blessed reference of the respective type.

Tie::Hash

In case of Tie::Hash, there is only CLEAR() that slowly clears the tied hash by iterating over all keys and removing them one after the other. Consequentially, both Tie::StdHash and Tie::ExtraHash implement faster variants that simply assign an empty list to the internal hash.

All of the other methods in Tie::Hash are just code that complains where perl would also complain without any extra code. use Carp, use warnings::register, and the methods new(), TIEHASH(), and EXISTS() can simply be removed from Tie::Hash.

Tie::Array

The methods UNSHIFT, SHIFT, CLEAR, PUSH, POP, and SPLICE, and EXTEND in Tie::Array are again generic methods that do not depend on the implementation.

The remaining methods EXISTS and DELETE in Tie::Array just complain, and DESTROY is redundant. They all should be removed.

Tie::StdArray implements standard array behaviour in short and efficient code, reimplementing most of the methods in Tie::Array with faster code.

Tie::Scalar

A scalar is so simple that it needs only three methods. All of them are primitive and can not be replaced by calls to other methods. Consequentially, Tie::Scalar contains only code to complain, all of it should be removed: All methods, use Carp, and use warnings::register. What remains is POD, the package Tie::Scalar, and our $VERSION. Moving Tie::StdScalar to Tie/StdScalar.pm breaks backwards compatibility, so use Tie::StdScalar should be added to Tie::Scalar. Also, Tie::StdScalar should keep inheriting from the empty Tie::Scalar.

Yes, it looks like nonsense. But it allows to add more methods to a tied scalar class in future perl versions, and having a sane default for the extra methods in either Tie::Scalar or Tie::StdScalar.

Tie::Handle

Tie::Handle once again contains some generic methods (PRINT and PRINTF use WRITE, GETC uses READ), and several complaining methods (TIEHANDLE, READLINE, READ, WRITE, CLOSE), as well as the useless new(). new() and the complaining ones have to go. Tie::StdHandle implements all standard methods, and a simpler version of GETC. No changes needed here, except for the wrong class name in the SYNOPSIS section of the POD.

Backwards compatibility needed?

I wanted to know which CPAN modules use one of the Tie::* classes, and which of those modules actually use the new() nonsense. https://grep.metacpan.org/search?size=20&q=Tie%3A%3A&qd=*&qft= runs into an HTTP 500 error. Is there a better way than downloading and unpacking the entire CPAN?

Alexander

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

Replies are listed 'Best First'.
Re^3: Breaking Tie::Hash into three modules
by afoken (Canon) on Sep 22, 2018 at 10:32 UTC

    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

    Preparing

    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/Scalar.pm, Tie/Array.pm, Tie/Hash.pm 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. Maintainers.pl 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.


    Notes

    (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.

    Alexander

    --
    Today I will gladly share my knowledge and experience, for there are no sweeter words than "I told you so". ;-)
Re^3: Breaking Tie::Hash into three modules
by Haarg (Curate) on Sep 12, 2018 at 11:10 UTC

    cpan.grep.me is usually better than grep.metacpan.org, although both tend to have issues with downtime.

    There are plenty of modules inheriting from these classes, although I haven't looked at how they are using them.

Log In?
Username:
Password:

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

How do I use this? | Other CB clients
Other Users?
Others lurking in the Monastery: (8)
As of 2020-07-13 09:02 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found

    Notices?