patternMinor
Sleep with count down display
Viewed 0 times
withsleepdowncountdisplay
Problem
I have a sleep function which displays the time remaining in sec, some feedback would be appreciated
This code and updates are available at csleep
use warnings;
$x=$ARGV[0];
$|++;
$n=0;
# arguments could be 1, 1s , 1m or 1h
if($x=~/([0-9]+)([a-zA-Z]?)/)
{
$t = lc($2) ;
if($t eq "h"){$n = $1*3600;}
elsif( $t eq "m" ){$n = $1*60;}
elsif( $t eq "s" || $t eq "") {$n = $1;}
else {die;}
}
else {die;}
while($n)
{
print $n;
sleep 1;
$len=length($n);
print "\r", " "x$len, "\r"; $n--;
}This code and updates are available at csleep
Solution
Naming
The names are very poor. Consider these renames:
Make the most out of regex
If you only allow "s", "m", or "h" as units, then instead of
Formatting
The formatting is really too compact. It's recommended to put spaces around operators.
It's recommended to
It can help catching bugs.
Alternative implementation
Applying the above suggestions (and a bit more), consider this alternative implementation:
The names are very poor. Consider these renames:
n->seconds
t->unit
x->input
Make the most out of regex
If you only allow "s", "m", or "h" as units, then instead of
[a-zA-Z] you could use [smhSMH] in the regex.Formatting
The formatting is really too compact. It's recommended to put spaces around operators.
use strictIt's recommended to
use strict always.It can help catching bugs.
Alternative implementation
Applying the above suggestions (and a bit more), consider this alternative implementation:
use warnings;
use strict;
my $seconds;
# input could be 1, 1s , 1m or 1h
my $input = $ARGV[0];
if ($input =~ /^([0-9]+)([smh]?)$/i) {
$seconds = $1;
my $unit = lc($2);
my $multiplier = 1;
if ($unit eq "h") {
$multiplier = 3600;
} elsif ($unit eq "m") {
$multiplier = 60;
}
$seconds *= $multiplier;
} else {
die;
}
$| = 1; # force flush output on every write
while ($seconds) {
print $seconds;
sleep 1;
my $len = length($seconds);
print "\r", " " x $len, "\r"; $seconds--;
}Code Snippets
use warnings;
use strict;
my $seconds;
# input could be 1, 1s , 1m or 1h
my $input = $ARGV[0];
if ($input =~ /^([0-9]+)([smh]?)$/i) {
$seconds = $1;
my $unit = lc($2);
my $multiplier = 1;
if ($unit eq "h") {
$multiplier = 3600;
} elsif ($unit eq "m") {
$multiplier = 60;
}
$seconds *= $multiplier;
} else {
die;
}
$| = 1; # force flush output on every write
while ($seconds) {
print $seconds;
sleep 1;
my $len = length($seconds);
print "\r", " " x $len, "\r"; $seconds--;
}Context
StackExchange Code Review Q#127456, answer score: 7
Revisions (0)
No revisions yet.