पर्ल - कोड संवर्द्धन

Question

मैंने अभी पर्ल में कोडिंग शुरू कर दी है और मैं यह पता लगाने के लिए देख रहा हूं कि नीचे दिया गया कोड अधिक कुशल बनाया जा सकता है या कम लाइनों में किया जा सकता है।

मैंने Win32::OLE मॉड्यूल और Text::CSV मॉड्यूल में थोड़ा सा शोध किया है, लेकिन यह अब तक जो मैंने पढ़ा है उससे जाने का तरीका प्रतीत होता है।

यह प्रश्न मूल रूप से एक नौसिखिया से पूछने वाला नौसिखिया है: "अरे, मैं बेहतर पर्ल प्रोग्रामर कैसे बनूं?"

कोड का उद्देश्य Excel कार्यपुस्तिका के निर्दिष्ट शीट में निर्दिष्ट श्रेणियों से डेटा प्राप्त करना और उन श्रेणियों की सामग्री CSV फ़ाइलों को लिखना है।

इसके अलावा, मुझे पता है कि मुझे सामान्य चेक लागू करने की आवश्यकता है, जैसे यह सुनिश्चित करना कि मेरा $cellValue इसे सरणी में जोड़ने से पहले परिभाषित किया गया है, और मैं समग्र संरचना के लिए और अधिक देख रहा हूं। जैसा कि सभी पूरी पंक्ति को एक बार में सरणी में डालकर लूपिंग को फ़्लैट करने का कोई तरीका है, या सरणी या संदर्भ में पूरी श्रृंखला या उस प्रकृति में से कुछ है? , इसके अलावा

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

करें कि आप अपने अनुक्रमित सही है बनाने:

धन्यवाद

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; 
     } 
    } 
} 

Answer 1

मार्क का सुझाव एक उत्कृष्ट है। एक और मामूली सुधार "नेस्टेड लॉजिक if $cell का एक गुच्छा" करने के लिए होगा, "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 एक अच्छी बात पर विचार करने के लिए, अपने डेटा को कभी भी सीएसवी मानक के आधार पर उद्धृत करने की आवश्यकता है जो इस पर निर्भर होगा (जैसे शामिल रिक्त स्थान, अल्पविराम , उद्धरण इत्यादि ...)। अगर इसे उद्धृत करने की आवश्यकता हो सकती है, तो पहिया का पुन: आविष्कार न करें, और इसके बजाय प्रिंटिंग के लिए 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 
    } 

Answer 2

कोई कारण नहीं है कि आंतरिक पाश है। मैं स्प्रेडशीट :: एक्सएलएसएक्स से परिचित नहीं हूं, इसलिए सुनिश्चित करें कि अधिकतम कॉल & पंक्ति शून्य के आपके कोड की तरह शून्य है। यदि वे नहीं हैं तो आप 0 .. $maxCol-1 से अधिक पुनरावृत्त करना चाहेंगे।

Answer 3

मैं हार्ड कोडिंग फ़ाइल नामों के खिलाफ सलाह दूंगा ... विशेष रूप से इस तरह की छोटी परियोजनाओं में, GetOpt::Long के माध्यम से फ़ाइल नामों को पारित करने की आदत प्राप्त करें। यदि आप अपनी सभी छोटी परियोजनाओं के साथ आदत से ऐसा करते हैं, तो यह एक बड़ी परियोजना पर निर्भर करता है जब इसे सही करने के लिए याद रखना बहुत आसान बनाता है।

आपका कोड अच्छी तरह से संरचित और पठनीय है, और आपने अपने लूप स्टेटमेंट्स की समस्याओं का अनुमान लगाया है, आपने चेतावनियां और सख्त उपयोग किया है, और आप आमतौर पर पुस्तकालयों का सही तरीके से उपयोग कर रहे हैं।

Answer 4

जैसा कि अन्य ने कहा है, आपका कोड स्पष्ट और अच्छी तरह से संरचित है। लेकिन मुझे लगता है कि इसे थोड़ी अधिक लापरवाही के साथ बेहतर किया जा सकता है।

निम्नलिखित बातों हैश या सरणी मूल्यों पर

• उपयोग शाब्दिक filehandles और open (open my $newfile, '>', $myFile) के तीन पैरामीटर प्रपत्र

• दोहराएं (या स्लाइस उनमें से) मन में आते हैं उनकी चाबियों या इंडेक्स की बजाय, जब तक आपको वास्तव में लूप

• डेटा के पॉइंटर्स निकालें एक पाश के भीतर ubstructures कि अगर पाश (my $rows = $worksheet->{Cells})

• स्पॉट जहां एक पाश का उपयोग कर रहे दूसरे में एक सूची को बदलने के लिए का ध्यान केंद्रित है, और map का उपयोग बजाय

मुझे आशा है कि मैं हेवन ' आपने प्रस्तावित के रूप में Text::CSV का उपयोग कर समाधान लिखकर बंदूक को थोड़ा सा कूद दिया। भाग्य के साथ यह आपके लिए निर्देशक है।

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"; 
    } 
}