Perl, how to make this scripter smaller, faster, better, stronger?

Question

I'm a newbie in perl, how I can make this better and faster? (SPOJ task)

until($i>99){

    $a=<>;

    if($a%5==0){
        if($a%3==0){
            print"SPOKOKOKO\n";
         }else{
             print"SPOKO\n";
         }
    }elsif($a%3==0){
        print"KOKO\n";
    }else{
        print"$a";
    }
$i++;
}

You should remove the trailing newline when reading from STDIN, use `chomp($a=<>)`. — Håkon Hægland, Sep 20 '20 at 19:35
So currently the script is working as you expect? You only want a review? Then you should rather ask at https://codereview.stackexchange.com/ — Håkon Hægland, Sep 20 '20 at 19:40
Faster is not necessarily easier to read. More elegant is not necessarily faster, nor easier to read. If you're interviewing for a job, solve for legibility first. If the interviewer would prefer you optimize for something else, clarify the tradeoffs that are acceptable. Also, avoid until. It's not heavily used, and would probably have points deducted for making the next person who comes along think about what it's doing versus while. — DavidO, Sep 21 '20 at 04:03
Also, this is almost the classic fizzbuzz problem, except that it doesn't count from 1-100, it iterates 100 times but each time waits on STDIN for another number to handle. I almost wonder if you misunderstood what was being asked, to have come up with code that will read from STDIN on each iteration. — DavidO, Sep 21 '20 at 04:06

Schwern · Accepted Answer · 2020-09-20T20:16:13.437

Something which is divisible by 5 and 3 is divisible by 15, so your logic is more straight forward as...

Divisible by 15
Divisible by 5
Divisible by 3
Not divisible by 5 nor 3

I'd also take advantage of say.

I'd also replace the awkward until with a for and not bother with $i.

Avoid using $a and $b because these are special variables used by sort.

Turn on strict and warnings.

Strip the newline off $a. Technically not necessary if you're going to use it as a number, but if you don't printing it will have an extra newline.

A non-number % anything is 0. Unless we validate the input, if you input nothing or "bananas" you'll get SPOKOKOKO. We can check if the input looks like something Perl considers a number with Scalar::Util::looks_like_number. If it isn't a number, throw an exception; it isn't the function's job to figure out what to do with a non-number. Perl doesn't really have exceptions, but we can get close with a die and eval.

And put the logic inside a subroutine. This separates the details of the input and output from the logic. The logic can now be named, documented, tested, and reused. This also avoids the redundant say.

use strict;
use warnings;
use Scalar::Util qw(looks_like_number);
use v5.10;

sub whatever_this_is_doing {
  my $num = shift;

  # Validate input early to keep the rest of the logic simple.
  if( !looks_like_number($num) ) {
    die "'$num' is not a number";
  }

  if($num % 15 == 0) {
    "SPOKOKOKO";
  }
  elsif($num % 5 == 0) {
    "SPOKO";
  }
  elsif($num % 3 == 0) {
    "KOKO";
  }
  else {
    $num;  # Note there's no need to quote a single variable.
  }
}

for(1..99) {
  my $num = <>;
  chomp($num);

  eval {
    say whatever_this_is_doing($num);
  } or say "'$num' is not a number.";
}

I'm sure there's some very clever math thing one can do to reduce the number of calls to %, but % is extremely unlikely to be a bottleneck, especially in Perl, so I'd leave it there.

With your first script (```for(1..100){$a=<>;if($a%5==0){if($a%3==0){print"SPOKOKOKO\n";}else{print"SPOKO\n";}}elsif($a%3==0){print"KOKO\n";}else{print"$a";}}```) i got 131 points, with new only 577 (smaller equals better) it's to complicated :c — Suchy, Sep 20 '20 at 20:18
@Suchy If there's some specific scoring system you're using, please include it in your question. I don't know what "SPOJ" is. This might be better on https://codegolf.stackexchange.com/ — Schwern, Sep 20 '20 at 20:19
I think the shortest code = better (in this scoring system). — Suchy, Sep 20 '20 at 20:24

score 0 · Answer 2 · answered Sep 20 '20 at 19:49

100 rows? That will be so fast that I don't understand why you are asking for optimization. Nevertheless...

The biggest two costs are reading 100 rows and writing 100 rows.

Then come the ifs and modulo checks -- each of these is insignificant.

One thing to consider is to turn it inside out. I'm suggesting 1 print with a messy expression inside the loop

print  ( ($a%5 == 0) ?
            ( ($a%3 == 0) ? "SPOKOKOKO\n" : "SPOKO\n" ) :
            ( ($a%3 == 0) ? "KOKO\n"      : $a        );

A lot fewer lines (3 vs 10); probably easier to read (especially if columns are aligned); perhaps faster.

Easier to digest. (I had not noticed that the %3 was s the same.) Oh, and now it is obvious that \n is missing from the last case; maybe a bug?

Perhaps some of the parentheses can be removed, but I don't trust Perl to 'do the right thing with nested uses of ?:.

Perl, how to make this scripter smaller, faster, better, stronger?

2 Answers2