One thing I see with your code is, that _set_Max_Questions() is called from two places. I would rather set $Max_Questions throughout the if blocks and afterwards (if the program is still alive) call _set_Max_Questions(). As an aside, this seems to be object oriented code - why do you use the form &_set_Max_Questions($self,...) instead of $self->_set_Max_Questions(...) ?

# First, handle the sensible default
$Max_Questions = $Total_Questions unless defined $Max_Questions;

# Now check for errors
if($Max_Questions > $Total_Questions){
  croak "Number of questions in $FileName exceeded";
  $Max_Questions = $Total_Questions;
}elsif($Max_Questions < 1){
  croak "Must have at least one question in the test.";
  $Max_Questions = 1;
}
# Now we have a valid number of questions
&_set_Max_Questions($self,$Max_Questions);
[download]

perl -MHTTP::Daemon -MHTTP::Response -MLWP::Simple -e ' ;    # The  
$d = new HTTP::Daemon and fork and getprint $d->url and exit;#spider
($c = $d->accept())->get_request(); $c->send_response( new   #in the
HTTP::Response(200,$_,$_,qq(Just another Perl hacker\n))); ' #  web
[download]

  $Max_Questions = $Total_Questions;
  &_set_Max_Questions($self,$Max_Questions);
[download]

The _set_Max_Questions function sets a parameter within the object that contains the number of questions that will be asked in the test. The $Total_Questions variable is the total number of questions that are in the question file that was passed when creating the actual object.

TStanley
--------
Never underestimate the power of very stupid people in large groups -- Anonymous

How about something like:

# default
$Max_Questions = $Total_Questions
  unless defined $Max_Questions;

# check boundaries
croak "Number of questions in $FileName exceeded"
  if $Max_Questions > $Total_Questions;
croak "Must have at least one question in the test"
  if $Max_Questions < 1;

# good to go
&_set_Max_Questions($self, $Max_Questions);
[download]

Update: D'oh! Corion beat me to it... but I like croak ... if better than if ... croak in this case; I think it makes it more obvious that the boundary checks are error conditions, rather than correctable "mistakes" (like undefined $Max_Questions).

Update 2: Fixed syntax error... thanks TStanley.

--
The hell with paco, vote for Erudil!
:wq

if($Max_Questions > $Total_Questions){
  croak"Number of questions in $FileName exceeded";
}elsif($Max_Questions < 1){
  croak"Must have at least one question in the test.";
}else{
  &_set_Max_Questions($self,(defined $Max_Questions ? $Max_Questions :
+ $Total_Questions));
}
[download]

Update This is just plain wrong, see post below.

                - Ant
                - Some of my best work - (1 2 3)

I don't find this obvious at all, as it behaves differently.

In the original code, if $Max_Questions is undefined, it gets a default value. In your code, if $Max_Questions isn't defined, you'll get two warnings, and it will croak with "Must have at least one question in the test.". The check on definedness of $Max_Question will not be reached if $Max_Question isn't defined.

I don't think there's much wrong with the original code, and I suggest leaving as it is.

Abigail

Ack... good point, my bad... I missed the obvious...

                - Ant
                - Some of my best work - (1 2 3)

One thing that nobody else has mentioned: the first if clause can be replaced with $Max_Questions ||= $Total_Questions. (Note, though, that this has different behavor then your code when $Max_Questions is defined, but false ("0" or ""). Your code will fall through to $Max_Questions<1, mine will use $Total_Questions instead.)

sub generate{
  my $self=shift;
  my $Total_Questions=_get_FileLength($self);
  my $FileName=_get_FileName($self);
  my $Data=shift;
  my $Max_Questions=shift;

  $Max_Questions = $Total_Questions
     unless defined $Max_Questions;

  croak"Number of questions in $FileName exceeded"
    if $Max_Questions > $Total_Questions;

  croak"Must have at least one question in test"
    if $Max_Questions < 1;

  &_set_Max_Questions($self,$Max_Questions);

#
# Original Code
# 
# if(!defined $Max_Questions){
#    $Max_Questions=$Total_Questions;
#    &_set_Max_Questions($self,$Max_Questions);
#  }elsif($Max_Questions > $Total_Questions){
#    croak"Number of questions exceeds the amount in $FileName";
#  }elsif($Max_Questions < 1){
#    croak"Must have at least one question in the test";
#  }else{
#    &_set_Max_Questions($self,$Max_Questions);
#  }
  
  my %Randoms=();
  my @Randoms=();
  my %Test_Questions=();
  my %Test_Answers=(); 
  my %Question_Lengths=();
  my $E;

  for(1..$Max_Questions){
    my $question_number=int(rand($Total_Questions)+1);
    redo if exists $Randoms{$question_number};
    $Randoms{$question_number}=1;
  }

  @Randoms=keys %Randoms;
  &_shuffle(\@Randoms);

  for(my $D=0;$D<$Max_Questions;$D++){
    $Test_Answers{$Randoms[$D]}=pop @{$$Data{$Randoms[$D]}};
    $Test_Questions{$Randoms[$D]} = $$Data{$Randoms[$D]};
  }
  
  foreach my $key(keys %Test_Questions){
    $E=@{$Test_Questions{$key}};
    $Question_Lengths{$key}=$E;
  }
  return \%Test_Questions,\%Test_Answers,\%Question_Lengths,\@Randoms;
}
[download]