current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

I tried to answer the question "How to print a hash in Perl, such that 3 key value pairs are printed on each line?" like this.

As I got an uncommented downvote, I'd like to ask: What's wrong with/How to improve:

use Modern::Perl;

my %h = ();
for (0..7) {
    $h{$_} = chr(65 + $_);
}
print %h, "\n";
my $cols = +$ARGV[0] || 5;
my $n = -$cols;
for my $key (keys %h) {
    print $key, ' => ', $h{$key}, 0 == ++$n % $cols ? "\n" : "\t\t";
}
print $n % $cols ? "\n------" : "------";
share|improve this question

1 Answer 1

up vote 3 down vote accepted

Wrong is too heavy word, and I don't why it get down voted, but minor improvements are possible.

  • Modern::Perl is not core module and it usually means strict, warnings, and perhaps autodie are used (plus some other tweaks).

  • chr() is not needed, and hash slice assignment works equally well, not to mention that it is perl idiom

  • keys %h may generate large in memory list for bigger hashes, and while .. each handles such cases better. There is a catch with each as it uses internal hash pointer which needs to be reset with keys %h if you want to start back from hash beginning (in non LIST context keys %h doesn't generate list).

use strict;
use warnings;

my %h;
@h{ 0 .. 7 } = "A" .. "H";
my $cols = 3;
my $n = -$cols;

keys %h;
while ( my ($k,$v) = each(%h) ) {
  print "$k => $v";
  print 0 == ++$n % $cols ? "\n" : "\t\t";
}
print "\n" if $n % $cols;
print "------";
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.