http://qs321.pair.com?node_id=279895


in reply to Re: Re: RFC: Seconds2English
in thread RFC: Seconds2English

Mostly, I was thinking: why does everyone want to reinvent wheels when learning? Why not find a wheel and augment it?

The process of augmentation will mean you have to understand the code and the rest of us get the benefit of an improved module.

You say your module has features above and beyond Time::Duration. So patch TD. I'm sure Sean would welcome your patch. Think of it as an exercise in community development.

As for the code: As belg says, look to something like Class::Accessor to obviate that nasty infection of identical accessors. I'd argue against using AUTOLOAD to create accessor. Something like chromatic's loop is much better.
You seem to do a lot of calculation when anything is changed, rather than just when people want things. I'd like to comment more, but your main problems are in your todo list - items 1, 2 and 3. POD is well covered by perldoc perlpod. Tests, you've been given a pointer, and there's an excellent section in "Learning Perl Objects, References and Modules", merlyn's new book. Comments you just need to make the code clearer. It's hard to read with all the references you're using (and, as they're references, you can assign them to temporary variables which (as they're references) affect the original when you modify them). I can't quite see where you work out how long a month is (a fun thing since it varies according to when your period starts). As far as I can tell, it's one of those hard coded large numbers. (sidenote: 31_536_000 is valid perl: the underscores are ignored). You take _is_number from Scalar::Util. Why? It was happy there. You should access your internal bits via methods rather than direct hash access. Makes it easier when I want to write my subclass, and means you can stave off calculations until the methods are invoked, rather than needing calculation up front to make sure that the hash values are always right (well, until I reach in and modify one and they're all wrong).

The general thing is that when code is appropriately commented, documented and tested, if it does what it's meant to do, it's good. Just like the rest of cpan (ha!)