On Friday 04 Dec 2009 06:10:16 Eric Mooshagian wrote: > Dear All, > > This is my first post. > > I present some subroutines that 1) create an index for one or more > arrays and then 2) get summary statistics based on the index. I have 2 > issues: >
First of all, based on a cursory look of your code, I should note that you may opt to use Statistics-Descriptive instead: http://search.cpan.org/dist/Statistics-Descriptive/ Caveat: I'm its maintainer on CPAN. > 1. I would like to be able to create the index conditional on the > values of another array, e.g., conceptually something like: > > my @index = &defindex(\...@a,\...@b,\...@c) for \...@dv >150 & dv < 800 I don't understand this code. Do you want to use perldoc -f map: http://perldoc.perl.org/functions/map.html ? A few notes: 1. Don't do ampersand-subroutine: http://www.perlfoundation.org/perl5/index.cgi?subroutines_called_with_the_ampersand 2. You have three arrays passed as a reference. Please consider using named arguments, in case you have too many parameters: <<<<<<<< print_details( { first => "John", last => "Doe", city => "Tel Aviv", . . . } ); >>>>>>> > > However, I have no idea how to write the code to do this. Ideally I > would be able to have several constraints (e.g., filter by reaction > time, accuracy, etc). Any clues? Is there a specific keyword or > topic I should search? I don't understand. You may look at some list operations from: 1. http://www.shlomifish.org/lecture/Perl/Newbies/lecture2/useful_funcs/ 2. http://search.cpan.org/perldoc?List::Util 3. http://search.cpan.org/dist/List-MoreUtils/ > > 2. This following section is redundant across the subroutines, but I > cannot figure out how put it into another subroutine and return the > result in a way that I can use it. I think I'm just not understanding > how to correctly return/use the hash with multiple values per key. ( I > want to maintain separate subroutines for each measure) > > my @ke...@{$_[0]}; > my @valu...@{$_[1]}; > my %combos = (); > > my $i=0; > foreach (@keys) { > my $key=$keys[$i]; > my $value=$values[$i]; > > push( @{$combos{$key}}, $value ); > $i++; > } What you're doing here is looping over the keys, discarding the values and just incrementing the key. I would write this as (untested): <<<<<<<<<<<<<<<<<< sub get_combos_from_kv { my ($keys, $values) = @_; my %combos; foreach my $i (0 .. $#$keys) { my $key = $keys->[$i]; my $value = $values->[$i]; push @{$combos{$key}}, $value; } return \%combos; } >>>>>>>>>>>>>>>>>> (There may be a more lispy/haskelly way using the functions in List-Util and List-MoreUtils, but this code is also OK.) And you call it like this: <<<<<<<<<<< my $combos = get_combos_from_kv(\...@keys, \...@values); >>>>>>>>>>> > > Finally, I have no doubt people can suggest how make the code > generally more perl like and efficient. > > Many thanks in advance for any help, > Eric > > > #!/usr/bin/perl > > use strict qw(vars subs); Why don't you have "use strict 'refs'" too? I.e: <<< use strict; >>> > use warnings; It's good that you have strict and warnings. > use List::Util qw(sum); > > #example data > my @a = qw(Sub1 Sub1 Sub1 Sub1 Sub1 Sub1 Sub2 Sub2 Sub2 Sub2 Sub2); > my @b = qw(left left left right right left right right left right > right); > my @c = qw(down up down down up up up down down up up); > my @dv= (.87,.2,.655,.7,.5,.3,.1,.45,.70,.900,.3); I should note that these decimal floating-point fractions will not be exact, due to their constraints as base-2 fractions. See: http://docs.sun.com/source/806-3568/ncg_goldberg.html (Which I haven't read yet, but am aware of several of its implications.) > my @acc= qw(c c c i c i c c c i c); > > > ########################################### > my @index = &defindex(\...@a,\...@b,\...@c); You should never use ampersand-subroutine-call: http://www.perlfoundation.org/perl5/index.cgi?subroutines_called_with_the_ampersand > > getmean(\...@index,\...@dv); > getmedian(\...@index,\...@dv); > getcount(\...@index,\...@dv); > getmean(\...@index,\...@acc); > > #define the index > sub defindex { > > my $i; > my $k; > my @combined; > my @meshed; > > for ( $i=0; $i<$#_+1; $i++) { $i < $#_+1 can be written as "$i < @_" which is short for "$i < scalar(@_)". This entire loop can be written as: <<< for my $i (0 .. $#_) >>> Other than that I should note that my personal preference is not to clobber the @_ with items and instead to pass them as an array reference. > push(@combined,${"array$i"}=$_[$i]); Why are you using symbolic references and varvarname here: http://perl.plover.com/varvarname.html You should use an array. > } > > @meshed = mesh(@combined); > > $j=0; > while (@meshed){ > my $merged = join('',splice(@meshed,0,$#_+1)); $#_+1 is better written as << scalar(@_) >> > $index[$j]=$merged; > $i++; > } You should use an array of array refs instead of one big array of concatenated fixed-length arrays. > > return @index; > > } #end defindex > You should return the @index as a reference. It's faster and cleaner. Don't clobber the return values. I'll try to comment on the rest of your code later on. > [SNIP] Regards, Shlomi Fish -- ----------------------------------------------------------------- Shlomi Fish http://www.shlomifish.org/ http://www.shlomifish.org/humour/ways_to_do_it.html Chuck Norris read the entire English Wikipedia in 24 hours. Twice. -- To unsubscribe, e-mail: beginners-unsubscr...@perl.org For additional commands, e-mail: beginners-h...@perl.org http://learn.perl.org/
