Beefy Boxes and Bandwidth Generously Provided by pair Networks
Perl: the Markov chain saw

Re^2: Climbing Mt. Perlcritic -- Switch statements

by puudeli (Pilgrim)
on Mar 26, 2009 at 07:03 UTC ( #753356=note: print w/replies, xml ) Need Help??

in reply to Re: Climbing Mt. Perlcritic -- Switch statements
in thread Climbing Mt. Perlcritic -- Switch statements

I would go exactly towards subroutines. Switch-statements are cluttered and at worst difficult to read and maintain. Given-when is similar although it does make the code a bit more readable. IMO the problem with switch/given structures is that too easily you insert multiple things into a clause making your logic/function "too long". It is also too easy to use overly complex conditions with given/when. In order to get cleaner and more testable code I would go for longer code. It eases the maintenance in the long run.

Without knowing the full code it is difficult to properly refactor it as there are too many variables whose context and details are unknown (eg. @PLACE, @batch, %files, is the code inside a function or not). But here is my humble try. I did make some assumptions about the code though. Correct me if I'm wrong.

# Introduce a new subs that can be tested. # New subs also separates the two functionalities # within the foreach loop. my @filenames = parse_file_names(@file_name_list); foreach my $file ( @filenames ) { if( $file =~ /go/ ) { populatePlaceIfExists(\%files, $file); @batch = (); } } sub parse_file_names { my @list = (); for( @_ ) { if ( /filename\s*=\s*(\S+)/x ) { push @list, $1; } } return @list; } sub populatePlaceIfExists { my $files_hash = shift; my $file = shift; if ( exists $files_hash->{$file} ) { push @PLACE, @batch; delete $files_hash->{$file}; } }

update: one function was missing, copy/paste error..

update2: I know I generated lots of code, the purpose is that most of the code is easily unit tested. Subs that are strongly decoupled from the data makes it easy to write unit tests. I'd like to know the context and the purpose of @batch and @PLACE so that I could see the associations between them and code and refactor even more.

seek $her, $from, $everywhere if exists $true{love};

Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://753356]
[Corion]: Yeah, I'm thinking more of marketing mailing lists, not public broadcast-style mailing lists
[marto]: yeah, so our hackerspace, we run mailman. that's a public discussion list, not a weekly buy our crap marketing list, people can unsubscribe at any time. What they can't do is delete their mails from the archive, or from the inboxes of our hundreds of user

How do I use this? | Other CB clients
Other Users?
Others chanting in the Monastery: (10)
As of 2018-05-22 08:20 GMT
Find Nodes?
    Voting Booth?