Скрипт для выполнения заданий с приобретенными XML-файлов


Я работал над сценарием в течение почти месяца теперь, которая принимает XML-файл и использует его, чтобы загрузить или загрузить файлы с FTP-сайта. Он также копирует файлы в каталог истории для исторических целей.

Это должно быть безупречным, поэтому какие-либо предложения, чтобы сделать скрипт более стабильным, надежным, безошибочным (должны уметь записывать ошибки и продолжать с окончанием сценария только потому, что один файл не найден). В конце концов, там будет функция электронной почты, чтобы позволить мне знать о любых ошибок и предупреждений. Я также будет меня обновления базы данных с информацией, но об этом позже вот этот скрипт и образец XML-файла.

На Perl

#! /usr/bin/perl -w
use DBI;
use strict;
use Switch;
use Net::FTP;
use Net::FTP::File;
use File::Copy;
use Getopt::Long;
use File::Basename;
use XML::Simple qw(:strict);
use Mail::Sender::Easy qw(email);

my ($e, $w, $p);
my (@files, @history);
my (%failed, %delete, %missing);
my ($config, $options, $xml, $ftp, $sbj, $msg, $error, $fi);

$options = GetOptions ("config=s" => \$config);
unless(-e $config){print "Could not find $config.\n";exit;}
$xml = XMLin($config, ForceArray=>0, KeyAttr=>[]);

if(exists $xml->{files}->{file}){
if (ref($xml->{files}->{file}) eq ""){
unless(-e $xml->{files}->{file}){$missing{$xml->{files}->{file}} = $!;next;}
if(lc($xml->{type}) eq "upload"){
    push(@files, "$xml->{localpath}/$xml->{files}->{file}");
}
elsif(lc($xml->{type}) eq "download"){
    push(@files, $xml->{files}->{file});
}
}
if(ref($xml->{files}->{file}) eq "ARRAY"){
if(lc($xml->{type}) eq "upload"){
    foreach $fi ((@{$xml->{files}->{file}})){
        unless(-e $fi){$missing{$fi} = $!; next;}
        push(@files, "$xml->{localpath}/$fi");
    }
}
elsif(lc($xml->{type}) eq "download"){
    foreach $fi ((@{$xml->{files}->{file}})){
        push(@files,$fi);
    }
}
} 
}
elsif(exists $xml->{files}->{regex}){
if(lc($xml->{type}) eq "upload"){
@files = glob("$xml->{localpath}/$xml->{files}->{regex}");
}
elsif(lc($xml->{type}) eq "download"){
@files = ();
}
}
else{print "No files specified\n";}

$ftp = Net::FTP->new($xml->{host});
if($@ ne ""){$e=1;}
unless ($ftp->login($xml->{user}, $xml->{password})){print $ftp->message;}
unless($ftp->cwd($xml->{serverpath})){print $ftp->message; exit;} 

switch ($xml->{type}){    
case "upload"{       
foreach my $file (@files){
    if(exists $xml->{name}){
        unless($ftp->put($file, $xml->{name})){$failed{$file} =        $ftp->message;next;}
    }
    else{
        unless($ftp->put($file)){$failed{$file} = $ftp->message;next;}
    }
    push(@history, $file) if(exists $xml->{historypath});
}
}
case "download"{
if(scalar(@files) > 0){
    foreach my $file (@files){
        unless($ftp->exists($file)){$missing{$file} = "not found on server.";next;}
        unless(exists $xml->{name}){$ftp->get($file, "$xml->{localpath}/$xml->     {name}");}
        unless($ftp->get($file, "$xml->{localpath}/$file")){$failed{$file} =    $ftp->message; next;}
    }
}
elsif(scalar(@files) == 0){
    @files = $ftp->ls($xml->{files}->{regex}) if($xml->{files}->{regex});
}
}
else{print "job type not defined.\n";}
}
$ftp->quit;
if( @history > 0 ){
if($xml->{date} == 0){
foreach my $h(@history){
    unless(copy($h, "$xml->{historypath}/")){$delete{$h} =  $!; next};
    unlink($h) if($xml->{del} == 1);
}
}
elsif($xml->{date} == 1){
foreach my $h(@history){
    (my $n, my $p, my $s) = fileparse($h, qr/\.[^.]*/);
    use POSIX qw(strftime);
    my $file = $n . "-" . strftime("%b-%e-%H-%M", localtime) . $s;
    unless(copy($h,"$xml->{historypath}/$file")){$delete{$h} = $!; next;}
    unlink($h) if($xml->{del} == 1);            
}

 }
 }

while ((my $k, my $v) = each %missing){print "$k => $v\n";}
while ((my $k, my $v) = each %failed) {print "$k => $v\n";}
while ((my $k, my $v) = each %delete) {print "$k => $v\n";}

В XML

<?xml version="1.0" encoding="ISO-8859-1"?>
<site>
    <job>zenadown</job>
    <type>download</type>
    <host>zena.leegin.com</host>
    <ip>128.222.79.75</ip>
    <user>leo</user>
    <password>green97</password>
    <serverpath>/leo</serverpath>
    <localpath>/temp</localpath>
    <historypath>/temp/leo</historypath>
    <files>
        <file>jo.png</file>
        <file>jo2.png</file>
        <file>jo3.png</file>
        <file>jo4.png</file>
    <file>Blogger.png</file>
    </files>
    <logfile>/zena/AUTOMIZE/leotest.txt</logfile>
    <del>0</del>
    <date>0</date>
</site>


331
4
задан 19 августа 2011 в 10:08 Источник Поделиться
Комментарии
5 ответов


  1. Пожалуйста, последовательно отступ. Это делает его легче для других, чтобы читать ваш код, и это делает его проще для вас , чтобы читать ваш код, предотвращая ошибки. Увидев трех брекеты в ряд выстроились вдоль левой стороны аэровокзала заставляет меня грустить.

  2. Не используйте переключатель. Использовать приведенный/когда, или использование if/elsif операторы сетей, как вы делаете в другом месте в коде, или использовать отправка таблицы (хеш, содержащий код ссылки), или использовать ОО методы называют по имени, а не использовать рубильник. Это делает ваш код сложнее отлаживать, и иногда это будет вызывать ваш код, чтобы просто перестать работать.

  3. Вы используете ForceArray => 0 , а затем переключиться на значениях вы найдете arrayrefs или простые скаляры, и писать много повторяющихся код, в ситуации, когда (насколько я могу сказать) вы могли бы просто использовать ForceArray => 1 и сбрить половину кода. Почему?

  4. Трудно сразу сказать, что код делает и почему, и это плохой знак для надежности. Попробуйте разбить его на функции вдоль функциональных линий-например, одну для анализа необходимую информацию из XML-файла, один для выполнения загрузки, одна для выполнения загрузки, и представить результаты пользователю. Дать каждой функции в документации, которая говорит, что он ожидает в качестве входных данных, что она производит как выход, как он ведет себя в случае ошибок и т. д. Программа, как ваша, должна иметь не более десятка строк кода вне функции. В самом деле, вы должны, вероятно, вдвое больше.

5
ответ дан 19 августа 2011 в 11:08 Источник Поделиться

Добавление к тому, что @Хоббс сказал:


  1. Отступ: вам perltidy , чтобы помочь форматировать ваш код.

  2. Переключатель против дается/когда: как долго, как ваш код использовать У5.10; дано/когда делает для лучшей альтернативой, чем переключатель модуль. Также, Когда также может быть полезен для петли, и я вижу некоторые места в коде, которые можно сделать более понятным, используя несколько петель.

  3. Модуль XML::простой не могут даже подойти. Этот модуль имеет простой интерфейс, но она предназначена больше для чтения XML-файлов конфигурации, чем то, что выглядит как выше XML-ответ. Возможно совмещение с XML::в libxml (для разбора на дом) и в xml::в libxml::XPathContext (для навигации что дом) будет лучше здесь, (не говоря уже о формате XML::в libxml быстрее, будучи построены на библиотеке libxml2.)

  4. Ваш сценарий похож на контроллер какой-то: он принимает на вход XML и решит, следует ли загрузить или скачать файлы, указанные в инструкции разбирается там. Может быть лучше разбить большой части в модули (например, модуль для разбора XML, а другой для загрузки/выгрузки файлов, и в итоге один рассыльщик/обработчик ошибок.)

1
ответ дан 2 сентября 2011 в 08:09 Источник Поделиться

Наиболее очевидное изменение вы должны сделать, это добавить некоторые комментарии и, если вы не имеете его в другом месте, какие-то документы!

Типовая документация предназначена для пользователя - это может быть конечный пользователь, или это может быть другой программист, который просто хочет использовать свой код как "черный ящик библиотека". Комментарии документация для программиста, который должен изменять и отлаживать код. Этот человек может быть вам несколько месяцев вниз по линии, или это может быть кто-то другой. Хорошие комментарии кратко описать алгоритм, поэтому они предоставляют дополнительную информацию для пользователя на уровне документации, которые должны содержать описание входных и выходных данных.

Даже думал, что это твой код и ты с нею близко знакомы, вы будете удивлены, как трудно это может быть чтобы понять, что он делает через несколько месяцев от него. Хорошие комментарии и документации смягчить это.

1
ответ дан 28 сентября 2011 в 10:09 Источник Поделиться

Использовать более описательные имена переменных. Видя глобальные переменные мою ($Е $З, $Р);
в верхней части вашего скрипта дал мне мандража. Используя в качестве счетчика в короткие петли могут быть приемлемыми, в противном случае вы должны использовать имена переменных, которые на самом деле описать то, что они содержат.

Как правило, я стараюсь писать полное описание кода, который я собираюсь написать, то берут имена переменных из этого описания.

1
ответ дан 14 апреля 2012 в 12:04 Источник Поделиться

Отдельные буквы почти никогда имена переменных.
дело в точку:
мой ($Е $З, $Р);

Нет ни одного комментария в этот скрипт, но этот скрипт является очень сложным и трудно выяснить. Попробую объяснить немного то, что вы делаете каждый раз.
Попробуйте разбить ее на несколько более мелких проблем и обратите внимание на эти проблемы. Часто делаешь что приводит к повторному использованию кода.
Нет дизайн это код, означающий, нигде я не вижу, что вы пытаетесь сделать, и потом, как вы собираетесь это сделать.
Отметив, что вниз будет легко читать обзор того, что вы делаете.
Что также будет очень полезно, если когда-нибудь решение должен быть реализован на другом языке программирования. Или если то, что вы делаете нужно объяснять никому...

0
ответ дан 20 июля 2012 в 11:07 Источник Поделиться