It's halfway there. Perl has a tradition of using a get/set accessor of the same name versus other language having separate get_foo and set_foo methods. But this is a different matter, since the underlying values are not simple scalars. If you want to keep accessor-only property access, you should use something like:

 # Dereference so that they can't go behind our backs
 sub fields { return @{ $self->{_fields} } }

 sub add_field {
     my ( $self, $field ) = @_;
     push @{ $self->{_fields} }, $field;
     $self->{_fields_hash}->{ $field } = 
                       scalar @{ $self->{_fields} } - 1;
 }

 sub remove_field {
     my ( $self, $field ) = @_;
     my $idx = $self->field_exists( $field );
     return unless ( $idx );
     delete $self->{_fields_hash}->{ $field };
     splice( @{ $self->{_$fields} }, $idx, 1 );
 }

 sub field_exists {
     my ( $self, $field ) = @_;
     return $self->{_fields_hash}->{ $field };
 }
[download]

(Using the index in as a return value for field_exists() is kind of peek-a-boo logic, but it's a start.)

Untested because I have something due in a few minutes, but you get the idea :-)

Chris
M-x auto-bs-mode