3
\$\begingroup\$

I'm a Perl beginner, I really love this language, and I was wondering if it was possible to reduce the size of this code, just for fun and so as to learn Perl specificities:

chomp($n=<STDIN>);
if (!$n) {
   print 0;   
} else {
   @t=split/ /,<STDIN>;
   foreach $i(0..$n-1) {
      $m=$t[$i]if(!$m)||(abs($t[$i])<abs($m))||(($t[$i]==-$m)&&($t[$i]>0));    
   }
}print $m;

The goal of this code is to give the closest number to 0 of a list, and if there are -4 and 4 for example, it will always return the positive number.

  • $n is the number of numbers in the list

  • @t is the list as an array

  • $m is the min

This is "Temperatures" game on the website Codingame.com.

This code is working pretty well, and I'm trying to reduce its size so as to discover some Perl secrets.

I have some precise questions:

  1. Is it possible to use the ternary operator with several instructions in Perl?
  2. Is is possible to write the foreach loop on one line in this case, or even replace the foreach loop with something else?

I don't really want perfect code as an answer, but some indications, and then I can search details and learn by myself.

\$\endgroup\$
0

2 Answers 2

4
\$\begingroup\$

A bad example

Perl is a language in which the shortest code is usually not the best code. For example, this short program accomplishes the task:

print((sort {abs($a)<=>abs($b) || $b<=>$a} (split ' ',<>)[0..<>-1])[0] // 0)

Not only is it hard to read, it's also algorithmically inefficient (because sorting is O(n log n) rather than O(n)). I don't recommend writing code like that.

Critique

You should use more whitespace. It won't make your code look any less clever, nor will it make your code run any slower.

You can safely dispense with chomp(), as Perl will naturally disregard the trailing newline when interpreting the string as a number. You could probably write <> instead of <STDIN>, unless you insist on reading only from standard input.

Counting loops are clumsy in Perl. When possible, opt for higher-level list-processing techniques. In this case, I recommend List::Util::reduce.

A possible solution

In Perl, there are usually many possible ways to solve a problem. Here's one short solution that is O(n), but avoids being the shortest possible solution. I'll let you figure out how it takes advantage of the <=> operator.

use List::Util qw(max reduce);
use strict;

my $n = <>;
my @temps = (split ' ', <>)[0 .. $n - 1];
print((reduce { ($a, max($a, $b), $b)[1 + (abs($a) <=> abs($b))] } @temps)
       // 0);

The special case of $n == 0 can be handled using a // logical defined-or operator. A || logical or operator would also happen to work, but it wouldn't be as appropriate.

Since the @t is meant to represent temperatures, renaming it to @temps would make the code less cryptic.

\$\endgroup\$
2
\$\begingroup\$

The other answer has good observations and provided a fine alternate solution. I'll add a few comments, sticking more closely to the code as implemented.

Strict and Warnings

The first item on every Perl code review is to make sure the code has:

use strict;
use warnings;

These help find bugs in your code.

warnings identifies a minor bug with this line:

}print $m;

Firstly, the brace does not belong on the same line as the print.

Secondly, the print statement should be inside the else block.

Whitespace

As mentioned in the other answer, the code is hard to read and understand because it lacks space around the operators.

Operators

The code would be easier to read using the and and or operators instead of && and ||, respectively.

foreach

Since foreach is identical to for, you can just shorten it to for .

split

The code would be more flexible if it allowed any number and type of whitespace in the split regular expression:

split /\s+/

Newlines

Customarily, you would always print a newline character, \n, when outputting information to the user.

Input

When running the code, the user has no idea what to do. Print out prompts for the user for each STDIN input.


Here is new code with the suggestions above:

use warnings;
use strict;

print "Enter the number of temperatures:\n";
chomp(my $n = <STDIN>);
if (!$n) {
   print "0\n";   
} else {
   print "Enter a list of $n temperatures, separated by spaces:\n";
   my @t = split /\s+/, <STDIN>;
   my $m;
   for my $i (0 .. $n-1) {
      $m = $t[$i] if (!$m) or (abs($t[$i]) < abs($m))
                     or (($t[$i] == -$m) and ($t[$i] > 0));
   }
   print "$m\n";
}
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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