Beefy Boxes and Bandwidth Generously Provided by pair Networks
Clear questions and runnable code
get the best and fastest answer

Re^2: One for the weekend: challenge (self-review)

by kyle (Abbot)
on Jun 10, 2008 at 21:51 UTC ( #691346=note: print w/replies, xml ) Need Help??

in reply to Re: One for the weekend: challenge
in thread One for the weekend: challenge

I've had some time to ponder my own work here, and I'm writing here to share my thoughts.

Let me start on the defensive: I wrote this tired and trying to finish in minimal time. I was shooting for functionality more than elegance or brevity.

Probably the biggest wart is using caller as a sort of flag parameter to the main words_for_phone. I made a words_only, and it does nothing but call words_for_phone. There, I have a check to see if that's where the call came from:

# If this was called from 'words_only', don't produce an individua +l digit my $words_only; { no warnings 'uninitialized'; my @call = caller 1; $words_only = ( $call[3] eq __PACKAGE__ . '::words_only' ); }

What I should have done instead is change the interface. The new convention would be that words_for_phone is called with an array reference and an optional flag second argument. It's not called many places, so this wouldn't be a difficult change.

If I'd made "my $out = []" instead be "my @out", I'd have saved myself some "@{ }" in a few places. Really the only place $out appears by itself is at the return, so that could be "return \@out". On the other hand, making it an array reference from the beginning indicates clearly that we're returning an array reference.

I think mine's the only solution that uses a package. I made it modular so I could test it more easily, though this certainly cost some lines of code to support it. I don't regret this decision at all. It was great to be able to just run the tests and see if I'd fixed what I was working on (and if I'd broken anything that worked before).

This was written incrementally, and it still has that feel to me. I wrote something to handle the easiest cases first (where the phone can be entirely translated to words) and then built stuff up around it to handle more difficult cases.

If this were production code and not a toy problem, it would benefit greatly from sanity checks in several places. Dictionary words should contain no spaces or digits. Don't call the one instance method as a class method and vice versa.

I welcome comments from any other monks who might have read what I wrote.

Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://691346]
and all is quiet...

How do I use this? | Other CB clients
Other Users?
Others pondering the Monastery: (6)
As of 2018-06-23 12:22 GMT
Find Nodes?
    Voting Booth?
    Should cpanminus be part of the standard Perl release?

    Results (125 votes). Check out past polls.