Wowzers.Ну, ваша главная проблема в том, что вы используете глобальные переменные.Как правило, вам следует прибегать к глобальным переменным только тогда, когда ... ну, никогда, действительно.И, конечно, не в простых случаях, подобных этому.
Если вы используете лексическую область видимости для своих переменных и передаете аргументы подпрограммам, вы никогда не заметите подобные проблемы.Например:
my $foo = process($bar);
sub process {
my $arg = shift;
my $value = ....;
return $value;
}
Теперь я не могу не заметить, что в каждом случае вы выполняете одно и то же open, так почему бы не включить это в свою подпрограмму.В качестве преимущества, вам не нужно беспокоиться о закрытии файловых дескрипторов, так как они закрываются автоматически, когда выходят из области видимости.
Не уверен, для чего нужна ваша переменная $last_char, поэтому я оставил ее каклексический.Я также ничего не сделал с кодом в вашей подпрограмме, кроме исправления зверского отступа.Заметные изменения в вашем коде:
- Использование
strict и warnings! - Используйте возвращаемое значение, возвращая значение переменной с лексической областью
- Передачааргументы подпрограмм
chop -> chomp.В принципе, вам никогда не следует использовать chop. - Использование списка базовых имен для построения имен файлов вместо повторения похожих имен
- Использование трех аргументов
open, с явным режимом и лексическим дескриптором файла.
Примечание: Вы должны никогда, никогда писать Perl-код без использования use strict; use warnings;.Нет смысла не использовать их: вы будете тратить больше времени только на поиск простых ошибок.
Примечание № 2: Непроверенный код
use strict;
use warnings;
my @seasons = ("Spring", "Summer", "Fall");
for my $season (@seasons) {
my $input = $season . "SIMS.dat";
my $output = $season . ".dat";
output_data($input, $output);
}
sub processFile {
my $file = shift;
open my $fh, '<', $file or die "$file: $!";
while (my $row = <$fh>) {
chomp $row; # NOTE: never use chop, use chomp instead
my @field = split(/\|/, $row);
for (my $i=0; $i<@field; $i++){
if ($field[$i] =~ /^ /) {
$field[$i] = " ";
} else {
$field[$i] =~ s/ *$//g;
}
}
my $new_data = join "|", @field;
if(@field == 15) {
$new_data .= "|0";
}
$new_data .= "\n";
}
return $new_data;
}
sub output_data {
my ($input, $output) = @_;
open my $fh, '>', $output or die "$output: $!";
print $fh processFile($input);
}
ETA: Теперь, глядя на ваш код подпрограммы, мне приходят в голову следующие оптимизации:
$new_data .= $field[$i] . "|";
....
my $lastchar = chop($new_data);
Нет.Вместо этого используйте join:
$new_data = join "|", @field;
Эта часть:
if ($field[$i] =~ /^ /) {
$field[$i] = " ";
} else {
$field[$i] =~ s/ *$//g;
}
... либо изменит первое поле на один пробел " ",если первый символ является пробелом, или он удалит пробелы из конца строки.Это действительно что вы хотите?Т.е. " foo" будет изменено на " " (пробел).
Я бы предположил, что вы ищете что-то вроде:
$field[$i] =~ s/^ *//;
$field[$i] =~ s/ *$//;
В этом случае вы можете просто сделать:
for (@field) {
s/^ *//;
s/ *$//;
}
Работает, как задумано, потому что $_ имеет псевдоним для каждого элемента в массиве, и они будут изменены регулярным выражением подстановки.Более подробное решение:
for my $value (@field) {
$value =~ s/^ *//;
$value =~ s/ *$//;
}
Или, что еще лучше, вы можете включить это в свой оператор split:
my $new_data = join "|", split /\s*\|\s*/, $row;
$new_data =~ s/^ *//;
$new_data =~ s/ *$//;
Или использовать регулярное выражение, которое, вероятно, будет дешевле:
$row =~ s/\s*\|\s*/|/g;
$row =~ s/^ *//;
$row =~ s/ *$//;
my $new_data = $row;