Perl эффективный код

Question

ради того, чтобы стать лучшим сценаристом, есть ли способ сделать следующий код меньше строк?

sub task1_2_2 {
  #Make sure syslog configuration is correct
  my @r3r = my @r4r = my @r5r = my @r6r = my @r7r = my @r8r = ("name = 67.176.10.200","facility-override = local3");
  my @r1r = ("name = 67.176.10.200","facility-override = local3","source-address = 67.176.255.1");
  my @r2r = ("name = 67.176.10.200","facility-override = local3","source-address = 67.176.255.2");
  my (@r1,@r2,@r3,@r4,@r5,@r6,@r7,@r8,%seen,@result1,@result2,@result3,@result4,@result5,@result6,@result7,@result8,$results,$item,$ii);
  my $pass = "pass";
  my $xinfo = shift;
  my $ip = shift;
  my $data = XML::LibXML->load_xml(string => $xinfo);
  my $datax = XML::LibXML::XPathContext->new($data);
  my $syspath = '/configuration/system/syslog/host/*';
  foreach ($datax->findnodes($syspath)) {
    if($ip eq "67.176.10.2" && $_->getName() ne "contents") {my $v = join " = ",$_->getName(),$_->to_literal;push @r1,$v};
    if($ip eq "67.176.10.3" && $_->getName() ne "contents") {my $v = join " = ",$_->getName(),$_->to_literal;push @r2,$v};
    if($ip eq "67.176.10.4" && $_->getName() ne "contents") {my $v = join " = ",$_->getName(),$_->to_literal;push @r3,$v};
    if($ip eq "67.176.10.5" && $_->getName() ne "contents") {my $v = join " = ",$_->getName(),$_->to_literal;push @r4,$v};
    if($ip eq "67.176.10.6" && $_->getName() ne "contents") {my $v = join " = ",$_->getName(),$_->to_literal;push @r5,$v};
    if($ip eq "67.176.10.7" && $_->getName() ne "contents") {my $v = join " = ",$_->getName(),$_->to_literal;push @r6,$v};
    if($ip eq "67.176.10.8" && $_->getName() ne "contents") {my $v = join " = ",$_->getName(),$_->to_literal;push @r7,$v};
    if($ip eq "67.176.10.9" && $_->getName() ne "contents") {my $v = join " = ",$_->getName(),$_->to_literal;push @r8,$v};
  }
  @seen{@r1} = ( ); if($ip eq "67.176.10.2") {foreach $item (@r1r) { push(@result1, $item) unless exists $seen{$item}; }}
  if(@result1) {
              foreach $ii (@result1) {$results .= "On R1 Syslog $ii ,is either missing or incorrect configuration was done\n";}
              }
  %seen=();
  $item=$ii = undef; 
  @seen{@r2} = ( ); if($ip eq "67.176.10.3") {foreach $item (@r2r) { push(@result2, $item) unless exists $seen{$item}; }}
  if(@result2) {
              foreach $ii (@result2) {$results .= "On R2 Syslog $ii ,is either missing or incorrect configuration was done\n";}
              }
  %seen=();
  $item=$ii = undef; 
  @seen{@r3} = ( ); if($ip eq "67.176.10.4") {foreach $item (@r3r) { push(@result3, $item) unless exists $seen{$item}; }}
  if(@result3) {
              foreach $ii (@result3) {$results .= "On R3 Syslog $ii ,is either missing or incorrect configuration was done\n";}
              }
  %seen=();
  $item=$ii = undef; 
  @seen{@r4} = ( ); if($ip eq "67.176.10.5") {foreach $item (@r4r) { push(@result4, $item) unless exists $seen{$item}; }}
  if(@result4) {
              foreach $ii (@result4) {$results .= "On R4 Syslog $ii ,is either missing or incorrect configuration was done\n";}
              }
  %seen=();
  $item=$ii = undef; 
  @seen{@r5} = ( ); if($ip eq "67.176.10.6") {foreach $item (@r5r) { push(@result5, $item) unless exists $seen{$item}; }}
  if(@result5) {
              foreach $ii (@result5) {$results .= "On R5 Syslog $ii ,is either missing or incorrect configuration was done\n";}
              }
  %seen=();
  $item=$ii = undef; 
  @seen{@r6} = ( ); if($ip eq "67.176.10.7") {foreach $item (@r6r) { push(@result6, $item) unless exists $seen{$item}; }}
  if(@result6) {
              foreach $ii (@result6) {$results .= "On R6 Syslog $ii ,is either missing or incorrect configuration was done\n";}
              }
  %seen=();
  $item=$ii = undef; 
  @seen{@r7} = ( ); if($ip eq "67.176.10.8") {foreach $item (@r7r) { push(@result7, $item) unless exists $seen{$item}; }}
  if(@result7) {
              foreach $ii (@result7) {$results .= "On R7 Syslog $ii ,is either missing or incorrect configuration was done\n";}
              }
  %seen=();
  $item=$ii = undef; 
  @seen{@r8} = ( ); if($ip eq "67.176.10.9") {foreach $item (@r8r) { push(@result8, $item) unless exists $seen{$item}; }}
  if(@result8) {
              foreach $ii (@result8) {$results .= "On R8 Syslog $ii ,is either missing or incorrect configuration was done\n";}
              }
  if($results) { return $results; } elsif(!$results) { return $pass }
}  

Я собираюсь написать много этих подпрограмм.По сути, я выполняю эту подпрограмму через цикл маршрутизаторов, и если что-то не соответствует тому, что я ожидаю, я хочу вернуть назад, какой маршрутизатор неверен, а что отсутствует.Код работает, но он очень многословен, так как я не очень давно его кодирую.Заранее спасибо за любые отзывы.

Answer 1

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

То, что выпрыгивает изменя больше всего повторяет использование жестко закодированных IP-адресов от 67.176.10.2 до 67.176.10.9.Вы связываете некоторые данные с каждым из них (например, @r1 и т.п.).Таким образом, вам бы посоветовали рассмотреть хеш.

"Но подождите, я могу только поместить скаляры в значение хеша!"Правда, вам нужно использовать ссылки ( учебник здесь ).

Вот пример того, как вы можете упростить эту первую большую подпрограмму:

sub task1_2_2 {
  # ...
  my %ip_address_to_r = (
      "67.176.10.2" => \@r1,
      "67.176.10.3" => \@r2,
      "67.176.10.4" => \@r3,
      "67.176.10.5" => \@r4,
      "67.176.10.6" => \@r5,
      "67.176.10.7" => \@r6,
      "67.176.10.8" => \@r7,
      "67.176.10.9" => \@r8,
  );

  # ...

  foreach ($datax->findnodes($syspath)) {
      next unless $_->getName() ne "contents";    # Go to next iteration of loop if we have "contents"

      my $v = join(" = ", $_->getName(), $_->to_literal);
      push @{$ip_address_to_r{$ip}}, $v;
  }

Обратите внимание на две вещи:

1. Мы создаем хеш, связывающий IP-адреса со ссылками на массивы (используя оператор ссылки \).Эта ссылка будет указывать на тот же массив, даже если он был изменен с помощью операций push.
2. В последней строке цикла for мы извлекаем ссылку как скаляр (поскольку скаляры являются ссылками),Затем мы «разыменовываем» ссылку, используя оператор @, который возвращает базовый массив.Тогда push работает так, как вы ожидаете.

Ищите способы использования ассоциативных массивов и ссылок в ваших интересах.Тогда вы можете выполнять поиск в гораздо меньшем количестве строк кода, и вам не нужно так много операторов if в ваших циклах.Также обратите внимание на общие условия, которые выполняются во всех случаях, и отделите их от внутренних условных выражений и поместите их в верхнюю часть цикла.