7

Perl でコーディングを始めたばかりで、以下のコードをより効率的にできるか、またはより少ない行数で実行できるかを調べています。

Win32::OLEモジュールとモジュールについて少し調べましたText::CSVが、これまで読んだことから、これが進むべき道のように思えました。

この質問は基本的に、初心者が年長者に尋ねるものです。

コードの目的は、Excel ブックの指定されたシートの指定された範囲からデータを取得し、それらの範囲の内容を CSV ファイルに書き込むことです。

また、配列に追加する前に my が定義されていることを確認するなど、一般的なチェックを実装する必要があることはわかって$cellValueいますが、全体的な構造をもっと探しています。行全体を一度に配列に入れたり、範囲全体を配列や参照に入れたり、その性質のものを入れたりして、ループを平坦化する方法はありますか?

ありがとう

use strict;
use warnings;
use Spreadsheet::XLSX;

my $excel = Spreadsheet::XLSX -> new ('C:\scott.xlsm',);
my @sheets = qw(Fund_Data GL_Data);

foreach my $sheet (@sheets) {

    my $worksheet = $excel->Worksheet($sheet);
    my $cell = $worksheet->get_cell(25,0);

    if ($cell) { # make sure cell value isn't blank
        my $myFile = "C:/$sheet.csv";
        open NEWFILE, ">$myFile" or die $!;

        # write all cells from Range("A25:[MaxColumn][MaxRow]") to a csv file
        my $maxCol = $worksheet->{MaxCol};
        my $maxRow = $worksheet->{MaxRow};
        my @arrRows;
        my $rowString;

        # loop through each row and column in defined range and string together each row and write to file
        foreach my $row (24 .. $maxRow) {

            foreach my $col (0 .. $maxCol) {

                my $cellValue = $worksheet->{Cells} [$row] [$col]->Value();

                if ($rowString) {
                    $rowString = $rowString . "," . $cellValue;
                } else {
                    $rowString = $cellValue;
                }
            }

            print NEWFILE "$rowString\n";
            undef $rowString;
        }
    }
}
4

4 に答える 4

6

マークの提案は素晴らしいものです。もう 1 つの小さな改善点は、"Do a bundle of nested logicif $cellを "do do not doに置き換えるunless $cellことです。この方法では、コードが少し読みやすくなります (余分なインデント/ネストされたブロックを 1 つ削除します。また、次の場合に何が起こるか心配する必要はありません)。 $セルが空です。

# OLD
foreach my $sheet (@sheets) {
    my $worksheet = $excel->Worksheet($sheet);
    my $cell = $worksheet->get_cell(25,0);

    if ($cell) { # make sure cell value isn't blank
        # All your logic in the if
    }
}

# NEW
foreach my $sheet (@sheets) {
    my $worksheet = $excel->Worksheet($sheet);
    next unless $worksheet->get_cell(25,0); # You don't use $cell, so dropped

    # All your logic that used to be in the if
}

ご指摘のとおりText::CSV、CSV 標準に基づいてデータを引用する必要があるかどうかに応じて (たとえば、スペース、カンマ、引用符などを含むなど)、検討することをお勧めします。引用する必要がある場合は、車輪を再発明せず、Text::CSV代わりに印刷に使用してください。テストされていない例は次のようになります。

# At the start of the script:
use Text::CSV;
my $csv = Text::CSV->new ( { } ); # Add error handler!

    # In the loop, when the file handle $fh is opened
    foreach my $row (24 .. $maxRow) {
        my $cols = [ map { $worksheet->{Cells}[$row][$_] } 0 .. $maxCol) ];
        my $status = $csv->print ($fh, $cols);
        # Error handling
    }
于 2012-05-24T20:40:30.473 に答える
6

その内部ループを持つ理由はありません:

print NEWFILE join(",", map { $worksheet->{Cells}[$row][$_] } 0 .. $maxCol), "\n";

また、インデックスが正しいことを確認してください。私は Spreadsheet::XLSX に慣れていないので、コードの残りの部分と同様に、最大列と行がゼロベースであることを確認してください。そうでない場合は、 を反復処理する必要があります0 .. $maxCol-1。

于 2012-05-24T20:33:27.233 に答える
4

ファイル名をハードコーディングしないことをお勧めします...特にこのような小さなプロジェクトでは、ファイル名を経由で渡す習慣を身につけてGetOpt::Longください。すべての小さなプロジェクトで習慣的にこれを行うと、より大きなプロジェクトが必要になったときに、それを正しく行うことを覚えやすくなります。

あなたのコードは適切に構造化されており、読みやすく、ループ ステートメントの問題を予測しており、警告と厳密を使用しており、ライブラリを一般的に正しい方法で使用しています。

于 2012-05-24T21:07:57.457 に答える
4

他の人が言ったように、あなたのコードは明確でよく構造化されています。しかし、もう少しパーリッシュで改善できると思います。

以下の点が気になります

  • レキシカル ファイルハンドルと 3 パラメータ形式のopen( open my $newfile, '>', $myFile)を使用します。

  • ループの本体にキーが本当に必要でない限り、キーやインデックスではなく、ハッシュ値または配列値 (またはそれらのスライス) を反復処理します。

  • ループの焦点である場合、ループ内のデータ部分構造へのポインターを抽出します ( my $rows = $worksheet->{Cells})

  • あるリストを別のリストに変換するためにループを使用している場所を特定し、map代わりに使用します

Text::CSVあなたが提案したようにusing を使用してソリューションを作成することで、銃を少し飛ばしていないことを願っています。運が良ければ、これはあなたにとって有益です。

use strict;
use warnings;

use Spreadsheet::XLSX;
use Text::CSV;

my $csv = Text::CSV->new;

my $excel = Spreadsheet::XLSX->new('C:\scott.xlsm',);

foreach my $sheet (qw/ Fund_Data  GL_Data /) {

  my $worksheet = $excel->Worksheet($sheet);
  next unless $worksheet->get_cell(25,0);

  my $myFile = "C:\\$sheet.csv";
  open my $newfile, '>', $myFile or die $!;

  my $rows = $worksheet->{Cells};

  # Write all cells from row 25 onwards to the CSV file

  foreach my $row (@{$rows}[24..$#{$rows}]) {
    my @values = map $_ ? $_->Value : '', @$row;
    $csv->print($newfile, \@values);
    print $newfile "\n";
  }
}
于 2012-05-25T03:21:31.043 に答える