Sorting array, getting modification of read-only value error

[myelinviolin]

Hey guys, I'm stuck with a sorting question. I keep getting the answer "Modification of a read-only value" at the sort. extract is a subroutine that extracts information from a file, and that is where the variables are defined ($name, etc.). I have successfully sorted things in the past, and have read (well, tried) a lot of things about this kind of error, but I am just not understanding the concept. The array prints out fine for me before and after the foreach loop, and I can't see any gaps in the information that would screw up a sort function. I am not defining anything in the extract subroutine as an array; everything is just a single variable. I've tried using strict and warnings and they didn't help with this problem. (should have used them from the beginning, but I didn't know how they worked and have gotten along fine without them so far). I was trying to use pop to put elements into the array, but the defining doesn't seem to be my issue here. What should I try looking at next in order to resolve this issue?

if($type eq 2){
@Files=<C:/perlscripts/Characters/*>;
$v=1;
foreach $file (@Files){
    $A=join "",(split /\//,$file)[3];
    #print "A:$A\n";
    $char[$x]=join "", (split /\./,$A)[0];
    $File_loc="C:\\perlscripts\\Characters\\";
    $name=$char[$x];

    $txt=".txt";
    $FILE=$File_loc.$name.$txt;
    #print "$FILE\n";
    open (CHAR_STATS, "$FILE")||die "Couldn't open the file, $!";
    extract(CHAR_STATS);

    $S[$v]=[$name,$INIT,$DEX+$INIT_MISC,roll("1d20")];


print ".$v.\t.$S[$v][0].\t.$S[$v][1].\t.$S[$v][2].\t.$S[$v][3].\n";
$v++;
}
#pop(@S);
print ".........\n";
$v--;
for($y=1; $y<=$v; $y++){
print ".$v.\t.$S[$y][0].\t.$S[$y][1].\t.$S[$y][2].\t.$S[$y][3].\n";
}
$a=(); $b=();
@G=sort{$a->[1] <=> $b->[1]||$a->[2] <=> $b->[2]||$a->[3] <=> $b->[3]}
+ @S;

for (my $i=0; $i<=$#G; $i++) { print "$G[$i][0]\t$G[$i][1]\t$G[$i][2]\
+t$G[$i][3]\n"; }
}

[tobyink]

I suspect autovivification.

Minimal example:

use strict;
use warnings;
my @sorted = sort { $a->[1] <=> $b->[1] } (undef, undef, undef);

[myelinviolin]

I think I get what you are trying to say, but I can print out the array with no problems, so the whole thing should be defined, correct? So I am not sure how I would go about fixing this.

[tobyink]

You can print out undefined values. (Though if you have warnings enabled, that kind of behaviour will earn you one.)

I'm not entirely sure how you'd fix it. Some self-contained code which doesn't rely on files from your system that I don't have on my computer might help.

[thundergnat]

What tobyink++ said. You are filling your @S array starting at index 1 ($v=1;). But arrays are (by default) zero indexed, so $S[0] has nothing in it. Sort then tries to auto-vivify values for $S[0] and gives you an error. Minimal "fix" is to index $v from 0.

You have many, many other sub-optimal things in there too though.

[myelinviolin]

This worked!! Thank you so much! This makes so much sense with the autovivication now. There WAS a gap in the array, so that's why it couldn't sort it. I know I do things differently than most people, but for me, readability and ease of use trump inefficiency any day. As long as the program runs without any wait periods, then it's ok with me.

[Anonymous Monk]

...I don't think the grandparent was talking about inefficiency when he said "sub-optimal"...

[GrandFather]

You really, really need strictures - always use strictures (use strict; use warnings; - see The strictures, according to Seuss). A quick glance through your code shows the following issues, at least some of which would be highlighted by 'strict':

1. use == for numeric compare, not eq ($type eq 2)
2. What is the scope of @char?
3. Where does $x get a value?
4. Use three parameter open and lexical file handles.
5. What is $a = () supposed to do?
6. Use Perl for loops rather than C for loops.
7. Use sane and consistent indentation.
8. Avoid global variables and side effects in functions (extract probably fails on both counts, but it's not clear from the calling code).

[myelinviolin]

I fixed the ==, got rid of $x. @char is used somewhere else where it actually is an array so I borrowed the code from that and the $x stuck in there, and it wasn't causing a problem so I left it. I was trying to use $a=(); to make the sort work, in case I had defined $a and $b earlier and it was messing up the sort. I'll look up the other point you made in #4 to help with file opening. I know all this probably annoys you guys, but I'm still learning how to keep my code neat. syntax, then logic, then neatness I guess is the order in my book. But, that's what the forum is for, so we can all learn. I'm very thankful for your help and input.

[GrandFather]

We are not at all annoyed to see someone trying, especially when they consider and make use of the advice they are given!

Actually I'd put neatness first because with a little practice syntax and logic both flow from it and if it is neat at least someone else may be prepared to give a hand with tricky bits.

There isn't enough context to show a "tidy" version of your code, but the following sample extrapolated somewhat from what you seem to be doing may be of interest. Note that the code below makes use of Perl objects to avoid global variables and to avoid domain specific knowledge leaking too much between different areas of the code.

#!/usr/bin/perl
use strict;
use warnings;
use Carp qw();

package Character;

sub new {
    my ($class, %props) = @_;

    # Validation of %props here as required
    Carp::confess "Characters require a 'name' property" if !$props{na
+me};

    return bless \%props, $class;
}

sub newFromStr {
    my ($class, $str) = @_;
    my $config = extract($str);

    return $class->new(%$config);
}

sub newFromFile {
    my ($class, $filename) = @_;

    open my $fin, '<', $filename or Carp::confess "Can't open $filenam
+e: $!\n";
    my $str = do {local $/; <$fin>};
    close $fin;

    return $class->newFromStr($str);
}

sub extract {
    my ($str) = @_;

    require 'YAML.pm';
    return YAML::Load($str);
}

# subs defining what a Character is here
sub Name {
    my ($self) = @_;

    return $self->{name};
}

sub SetAttributes {
    my ($self, %attribs) = @_;

    for my $attrib (keys %attribs) {
        my ($rolls, $die) = $attribs{$attrib} =~ /(\d+)d(\d+)/i or nex
+t;

        $self->{$attrib} = 0;
        $self->{$attrib} += int(1 + rand($die)) for 1 .. $rolls;
    }
}


package Game;

sub new {
    my ($class, %props) = @_;

    # Validation of %props here as required
    return bless \%props, $class;
}

sub AddCharacter {
    my ($self, $character) = @_;
    my $name = $character->Name();

    Carp::confess "Character name must be unique. $name is already in 
+use!"
        if exists $self->{characters}{$name};

    $self->{characters}{$name} = $character;
}

sub LoadCharacters {
    my ($self, $source) = @_;

    opendir my ($scan), $source or Carp::confess "Can't open $source: 
+$!\n";
    my @files = grep {-f $_} map {"$source\\$_"} readdir $scan;
    closedir $scan;

    foreach my $file (@files) {
        my $character = Character->newFromFile($file);

        $character->SetAttributes(hp => '1d20');
        $self->AddCharacter($character);
    }
}

sub DumpCharacters {
    my ($self) = @_;
    my $format = "%-15s %5s %5s %5s\n";

    printf $format, qw(Name init dex hp);

    for my $name (sort keys %{$self->{characters}}) {
        printf $format, $name, @{$self->{characters}{$name}}{qw(init d
+ex hp)};
    }
}

package main;

my $testFolder = "./Characters";

createTestCharacters($testFolder);

my $game = Game->new();

$game->LoadCharacters($testFolder);
$game->DumpCharacters();

sub createTestCharacters {
    my ($folder) = @_;
    my @characters = (
        {name => 'Red',          init => 5,  dex => 10},
        {name => 'Spotty',       init => 8,  dex => 15},
        {name => 'Night Runner', init => 15, dex => 17},
        );

    require 'YAML.pm';

    for my $character (@characters) {
        my $name = $character->{name};

        YAML::DumpFile("$folder\\$name.yaml", $character);
    }
}

Prints:

Name             init   dex    hp
Night Runner       15    17    14
Red                 5    10    16
Spotty              8    15    10