質問の0%を受け入れたという事実にもかかわらず(その数を増やすことは非常に公平です)、次回はより良い (そして機能する) Perl を作成できるように、ここにコードの分析を示します。
Perl の作成に関する一般的なコメント
規律とカオス
特に、スクリプトが機能しない理由を理解しようとしている場合や、Perl を初めて使用する場合は、常にuse strict
とを使用してください。use warnings
通常、これらのプラグマは、誰もが時々犯す最もばかげたエラーについて警告します。
Perl は、良いコード (および TIM TOWDTY) を書くことを強制しませんが、よほどの理由strict
がない限り、ほとんどの場合、Perl のサブセットに固執する必要があります。
Usingは、非常に正当な理由がない限り、strict
すべての変数を で宣言する必要があることを意味します。my
変数を宣言することは良いことだと思います。
Perl は C ではありません
ほとんどの組み込み関数は、引数を区切るために括弧を必要としません。すなわちsplit(/ /, $check)
とsplit / /, $check
は、ほとんどの状況で同じものです。
また、for(INIT; COMPARE; INCREMENT)
特に最後の部分が$i++
. foreach
代わりに、 -syntax と range を使用できます。
for my $i ($MIN .. $MAX)
コードが機能しない理由
そして:避けることができる悪いイディオムは何ですか
my
すべての変数は with で宣言する必要があり、より優れたループ構文が利用できることは既に指摘しました。
@file2 = @file // full content of the file in an array
//
コメントを紹介しません。これは定義された or演算子です。また、このステートメントは . で終了していません;
。for
次のループは同じステートメントの一部であるため、これは Perl を混乱させますが、これは無効です。
また、@file2
またはの内容を変更しない@file
ため、コピーを作成する必要がありません。
$temp = $file[$i];
あなたは決して使用しません$temp
。
@grepNames = grep(/$fields[2]/, @file2);
これを使用しgrep
て、同じ変数名を含む行が何行あるかを調べます。後ですべての要素をループするので、これは不要です。@file
if($#grepNames >= 1)
あなたが書いたのは質問です:最高指数は@grepNames
1以上ですか?あなたはおそらく、複数の一致があったことを意味していましたか? . Idはそれを次のように書きます
if (@grepNames > 1)
ただし、これは主にスタイル上のコメントです。
if( $file[$i] =~ /INTEGER/ ...
待って何?$file[$i]
が含まれている場合はINTEGER
、$check
またはも含まれます$temp
。配列添字の代わりにスカラーを使用すると、入力の手間を省くことができます。
push(@data, $file[$j]);
@data
同じ変数名の行が既にある場合でも、何かをプッシュします。さらに悪いことに、n 個@file
の要素が含まれている場合、内側のループはn - 1 個の要素を反復し、ほとんどの場合、何かをプッシュして、アルゴリズムをO(n²)にします。for
@data
i \ j | INTEGER | Int32 | UInt32
--------+---------+-------+-------
INTEGER | - | j | j
Int32 | i | - | j
UInt32 | i | i | -
これは、(ルールに従って) どの要素が に到達するかを示す表です@data
。この表をif
/と比較elsif
して、いくつかのケースが欠落していないかどうかを確認することをお勧めします。
push(@data, $file[j]);
$
前にシジルを忘れましたj
。
より良い解決策
あなたのアルゴリズムはO(n²)、またはむしろO(n * (2n - 1))で実行されます。ただし、問題はO(n)で解決できます。
私は問題を次のように見ました:
入力の各行には、識別子と関連する重みがあります。
出力については、同じ識別子を持つすべての行のセットから最小の重みを持つ行が選択されます。2 つ以上の行の最小ウェイトが同じ場合、これらの最小行のいずれかを選択できます。
行の重みは、行の 2 番目の単語がどのキーワードであるかによって異なります。
Unsigned32 => 1,
Integer32 => 2,
INTEGER => 3,
2 番目の単語がこれらのキーワードのいずれでもない場合、エラーがスローされます。
各識別子の最初の出現順序は、入力と出力の両方で同じにする必要があります。
私のソリューション (以下に示す) では、1 番目と 3 番目の単語を識別子として使用しました。行が出力に既に存在する場合、現在の行の重みが小さければ更新しました。
編集:私の解決策
#!/usr/bin/perl
use strict; use warnings;
my @data;
my %index;
while (<DATA>) {
$_ =~ s/^\s+//;
my @fields = split /\s+/, $_, 3;
@fields = (@fields[0 .. 1], split /(?=\W)/, $fields[2], 2);
my ($class, $type, $name, $rest) = @fields;
if (defined $index{$class}{$name}) {
# we have a predecessor
my $index = $index{$class}{$name};
$data[$index][1] = (sort compareTypes $data[$index][1], $type)[0];
} else {
push @data, \@fields;
$index{$class}{$name} = $#data;
}
}
foreach (@data) {
my @fields = @$_;
print "@fields[0..2]$fields[3]";
}
sub compareTypes {
my %weight = (
Unsigned32 => 1,
Integer32 => 2,
INTEGER => 3,
);
my $weight_a = $weight{$a} // die "undefined type $a";
my $weight_b = $weight{$b} // die "undefined type $b";
return $weight_a <=> $weight_b;
}
__DATA__
typedef INTEGER Id;
typedef Integer32 Id;
typedef Integer32 Id;
typedef Integer32 Id;
typedef INTEGER Identifier;
typedef Integer32 Index;
typedef Unsigned32 Identifier;
typedef Integer32 Index;
typedef Unsigned32 TunnelId;
typedef Unsigned32 TunnelId;
const Unsigned32 maxValue = 65535;
const Integer32 Index_maxValue = 65535;
const Unsigned32 maxValue = 4294967295;
const Unsigned32 Index_maxValue = 65535;
出力:
typedef Integer32 Id;
typedef Unsigned32 Identifier;
typedef Integer32 Index;
typedef Unsigned32 TunnelId;
const Unsigned32 maxValue = 65535;
const Unsigned32 Index_maxValue = 65535;