[Jingle]

I am simply trying to test the values in 2 arrays to see if they are the same.
the next step tho is to test each of them with each value in guesse with each value in rannum.

the problem is that no matter what I cannot get $correct to be more than 1 or 0; it almost seems to be acting like a bool.

sub TestCorrect  #tests for the correct numbers in the correct order
{
	my $correct = 0;
	@guesse = split " ,_-/", $guesse;
	print"@guesse\n";
	if($guesse[0]==$rannum[0]){ $correct++}
	if($guesse[1]==$rannum[1]){ $correct++}
	if($guesse[2]==$rannum[2]){ $correct++}
	if($guesse[3]==$rannum[3]){ $correct++}
	print"$correct\n";
	return $correct;
}

thanks for any help

This post has been edited by Jingle: 07 June 2012 - 02:38 AM

[dsherohman]

You should turn warnings on (with use warnings;), especially when you're trying to identify something that seems like a weird problem to you. I added a little extra to your posted code in order to produce a complete, runnable program and came up with this:

#!/usr/bin/env perl    

use strict;
use warnings;

my @rannum = (6, 8, 3, 10);
my $guesse = '12 8 9 10';

TestCorrect();

sub TestCorrect    #tests for the correct numbers in the correct order
{
  my $correct = 0;
  my @guesse = split " ,_-/", $guesse;
  print "@guesse\n";
  if ($guesse[0] == $rannum[0]) { $correct++ }
  if ($guesse[1] == $rannum[1]) { $correct++ }
  if ($guesse[2] == $rannum[2]) { $correct++ }
  if ($guesse[3] == $rannum[3]) { $correct++ }
  print "$correct\n";
  return $correct;
}

Running it produced the following output:

$ ./guesse 
12 8 9 10
Argument "12 8 9 10" isn't numeric in numeric eq (==) at ./guesse line 16.
Use of uninitialized value in numeric eq (==) at ./guesse line 17.
Use of uninitialized value in numeric eq (==) at ./guesse line 18.
Use of uninitialized value in numeric eq (==) at ./guesse line 19.
0

Notice the warnings I got: The first test complained that the argument "12 8 9 10" isn't numeric and the other three tests complained about uninitialized values - the entire string $guesse ended up in $guesse[0] and the rest of the @guesse array is empty!

Looks like the split didn't do what you intended. Since you gave it a string to split on, it's looking for the entire sequence " ,_-/" as the delimiter between values. I suspect you want it to split on any one of those characters, not to require all of those characters (and require them in that order). What you want to use there instead is a regular expression with a character class, which will match any one of the characters:

my @guesse = split /[- ,_\/]/, $guesse;

With this change, I now get the output:

$ ./guesse 
12 8 9 10
2

This correctly reports that two of the numbers match (the 8 and the 10).

Now that it's working, time for the critique. Looking at the code you posted, I see three problems with it. Two make it hard to reuse and the third is just excessive repetition:

1) By accessing @rannum and $guesse as global variables, you made it impossible to use this sub to compare data held in any other variables. This also sets you up for potential problems down the road if something else requires you to change how those variables are initialized and used. It's much better to pass the data you need into the sub as parameters instead of relying on globals.

2) It's generally better practice to separate calculations from input and output operations. You'll probably want to use your code without printing the result at some point (even if only to do automated tests), so I'd recommend moving the print out of this sub and into the calling code.

3) DRY. (Don't Repeat Yourself) You've got four lines that are identical aside from an array index. That's generally a sign that you should use a loop instead. A loop will make it more compact, plus, as a side bonus, it will allow you to deal with data sets whose size isn't known until the program runs.

With all those things in mind, as well as a bit of style tweaking, here's how I would write that:

#!/usr/bin/env perl    

use strict;
use warnings;

my @rannum = (6, 8, 3, 10);
my $guesse = '12 8 9 10';

print "$guesse\n";
my $score = calc_score(\@rannum, $guesse);
print "$score\n";

print "---\n";

my @another_key = (1, 2, 3, 4, 5, 6, 7, 8);
print calc_score(\@another_key, '1 1 3 6 5 5 7 8') . "\n";
print calc_score(\@another_key, '2 1 3 9 8 7 6 5') . "\n";

sub calc_score {
  my ($key_ref, $answer_str) = @_;
  my @key = @$key_ref;

  my $correct = 0;
  my @answers = split /[- ,_\/]/, $answer_str;

  for my $i (0 .. $#key) {
    $correct++ if $answers[$i] == $key[$i];
  }

  return $correct;
}

(Note that I'm assuming that you're scoring a test here, by comparing an answer key to a student's list of answers, so I renamed the sub and variables to whatever made the most sense to me in that context. Your actual use case and feeling for what names are most sensible may vary.)

[Jingle]

for a small amount of code that was a very long reply. Thank you I feel like I learned something. I had gotten it working but now I have some other Ideas to brighten my code with. those warnings are useful to.