search

Dual-Life / Time-Piece

Object Oriented time objects
Other
15 stars 33 forks source link

Clean inheritance #12

Closed dolmen closed 9 years ago

dolmen commented 9 years ago

Use Exporter and DynaLoader without inheriting from them. Because they pollute the namespace of our class. Because an empty @ISA makes methode lookup faster.

smith153 commented 9 years ago

Looks good thanks. Always wondered why it was the way it was...

smith153 commented 9 years ago

We might have broken subclassing/inheritance somehow. Module::New fails to build and test with Time::Piece-1.29.04:

t/00_load.t ......... 1/50 Failed test 'use Module::New::Date;' at /home/user/perl5/lib/perl5/Test/UseAllModules.pm line 71. Tried to use 'Module::New::Date'. Error: "gmtime" is not exported by the Module::New::Date module "localtime" is not exported by the Module::New::Date module Can't continue after import errors at /home/user/perl5/lib/perl5/x86_64-linux/Time/Piece.pm line 141. BEGIN failed--compilation aborted at /home/user/perl5/lib/perl5/Test/UseAllModules.pm line 71. Bailout called. Further testing stopped: failed: Module::New::Date FAILED--Further testing stopped: failed: Module::New::Date

Haven't looked into it yet, but Module::New builds fine with 1.29

dolmen commented 9 years ago

I'll investigate. On first quick look, I think I'll propose a patch to Module::New. I'll meet its maintainer in the next week at the Perl QA Hackathon.

smith153 commented 9 years ago

Yea, it could go either way. Wasn't sure which you would prefer.

dolmen commented 9 years ago

I think that we will have to restore inheritance from Exporter, as I see the way in which Module::New::Date use Time::Piece shows me some valid (but tricky) use cases that are now broken. I still have to find if they are really in the wild.

About Module::New::Date, I still think it will be much cleaner with my charsbar/module-new#2 fix.

smith153 commented 9 years ago

Another issue is the export method has: $class->SUPER::export Without inheritance, not sure where the call to SUPER will go.

smith153 commented 9 years ago

It looks like in version 1.09 the overridden export method was added with comment "Time::Piece should now be safely subclassable"

dolmen commented 9 years ago

I recomment to revert b0a7e5a787e2300dda618c303f9044332bd260f6 for now, because I broke ->import with that change. I will provide an alternate implementation later.