7
\$\begingroup\$

How could I improve this working Perl script, which changes my desktop background to the next available one, without opening a terminal? Specifically, how could I iterate over my .fehbg and walls/ lazily, without reading them into arrays? I couldn't find a way to do that; I need the for loops.

Other than that, are there any inefficiencies in my program, which could be spotted by a more experienced programmer?

#!/usr/bin/perl
# A background switching script which can be run from dmenu

use 5.18.2;
use strict;
use warnings;
use File::Basename;

my @fehbg = ();
my @files = ();
open 'FEHFH', '</home/chummer/.fehbg';
while(<FEHFH>){
    @fehbg = split/'/;
}
close FEHFH;
my $current = basename($fehbg[-2]);
opendir WALLSDIR, '/home/chummer/pictures/walls';
foreach(readdir WALLSDIR){
    unless($_ eq '.' || $_ eq '..'){
    push(@files, $_);
}
}
closedir WALLSDIR;
for(my $i = 0; $i <= $#files; ++$i){
    if($current eq $files[$i]){
    if(exists($files[$i+1])){
        foreach($files[$i+1]){ s/\s+/\\ /g; }
    system"feh --bg-fill /home/chummer/pictures/walls/$files[$i+1]";
    last;
    }
    else{
        foreach($files[0]){ s/\s+/\\ /g; }
    system"feh --bg-fill /home/chummer/pictures/walls/$files[0]";
    last;
    }
}
}
\$\endgroup\$
3
\$\begingroup\$

Use while to iterate over directory elements and skip reading them all at once into array,

opendir my $WALLSDIR, '/home/chummer/pictures/walls';
while (my $file = readdir($WALLSDIR)) {
    next if $file eq '.' or $file eq '..';

    # ..
}

Since you're not checking if open() or opendir() calls were successful at all, put

use autodie;

at beginning of your script, and exceptions with description will be thrown when some of these calls fails.

Stylistic remark, my @fehbg = (); is same as just my @fehbg;

Use lexical file-handles together with three argument open (also lexical dir-handle for opendir),

open my $FEHFH, '<', '/home/chummer/.fehbg';

Again stylistic remark,

for my $i (0 .. $#files)

is a more readable/simpler equivalent of,

for (my $i = 0; $i <= $#files; ++$i)

Then foreach($files[$i+1]){ s/\s+/\\ /g; } is longer and less performant version of $files[$i+1] =~ s/\s+/\\ /g;

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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