in reply to bin2dec for big numbers

Starting out with a value and modifying it piecemeal as you go through a list of something, which you do with $ret and the digits in both functions, is an ideal candidate for List::Util's reduce function.

use List::Util qw( reduce ); sub bin2dec { my ( $str ) = @_; reduce { mul2( $a, $b ) } "", split //, $str; }

Here, in the first iteration, $a is the empty string from the list passed to reduce, and $b is the first digit. The return value of the block is passed back to the block as $a in the next iteration, while $b goes through the all the remaining values from the list one by one.

$add_one_f I'd rename as $have_overflow, which documents intent rather than implementation. And if you only use it as a boolean, you don't need to initialize it either.

The for(;;) loop isn't necessary, you don't use the index anywhere, you just extract each corresponding character once. So you could rewrite that much more clearly as for( reverse split //, $str ). Then it could be turned into a reduce in the same way as above, but since you're just accumulating characters onto the front of a string, you are better served with a join reverse map.

sub mul2 { my ( $str, $have_overflow ) = @_; my $ret = join '', reverse map { my $c = $_ * 2 + ( $have_overflow ? 1 : 0 ); $have_overflow = ( $c >= 10 ); ( $c % 10 ); } reverse split //, $str; ( $have_overflow ? 1 : '' ) . $ret; }

You could even get rid of the out-of-band overflow check at the bottom if you rearrange things a little:

sub mul2 { my ( $str, $have_overflow ) = @_; join '', reverse map { my $c = $have_overflow ? 1 : 0; $c += $_ * 2 if length; $have_overflow = ( $c >= 10 ); # return a zero char only if not at top of string ( $c % 10 ) or ( length() ? 0 : '' ); } reverse '', split //, $str; }

But in this case I find it obfuscates more than it reduces redundancy.

Incidentally, the ( $have_overflow ? 1 : '' ) above is a no-op if $have_overflow is a boolean like in this case, since 1 and the empty string are what the boolean ops return in Perl for true and false respectively. So that could be written $have_overflow . $ret, though that lacks the the benefit of documented intent.

Makeshifts last the longest.

Replies are listed 'Best First'.
Re^2: bin2dec for big numbers
by spurperl (Priest) on Jan 18, 2005 at 13:03 UTC
    Thanks for the insightful reply.

    reduce definitely makes the code much clearer. I wonder how it is in terms of performance versus the bare loop.

    I wholeheartedly agree with the variable name change you are proposing. has_overflown makes the code clearer.

    Regarding changing the for loop with index to one w/o an index but on reverse, I just have a performance concern. reverse probably takes linear time in string length, and with an explicit for loop that goes from the end of the string to its beginning the cost is saved.

    Also, in your proposed implementation of mul2 with join reverse map I'm not sure if it's indeed clearer than the for loop. Again, the issue of performance rises, since you have two reverses extra.

      ObCounter: if you're so concerned about speed, why are you doing numerics in Perl? :-) In case of the reverse join, I don't see why your code is better anyway: you are constantly replacing the string by a character+string.

      In any case, it could probably be sped up a little by using string reverse vs list reverse:

      sub mul2 { my ( $str, $have_overflow ) = @_; my $ret = reverse join '', map { my $c = $_ * 2 + ( $have_overflow ? 1 : 0 ); $have_overflow = ( $c >= 10 ); ( $c % 10 ); } split //, scalar reverse $str; ( $have_overflow ? 1 : '' ) . $ret; }

      Makeshifts last the longest.