重構現有的perl子文件

Question

我在我的Perl代碼中有2 sub，它們幾乎完全相同。我很好奇，怎麼會重構它們來製作一個sub。

他們唯一真正的區別是一個正則表達式，並通過準備好的語句進行查詢。準備的語句也採用不同的參數。

想法？

sub showcaseViewsSubData { 
    my ($api_call, $stat_section, $idsite, $prev_date, $last_of_month, $params, $subtable) = @_; 

    return unless ($subtable); 

    my %sub_params = %{ clone ($params) }; 
    $sub_params{'idSubtable'} = $subtable->{'idsubdatatable'}; 

    # $data contains views for each primary showcase page 
    my $data = &fetchStatsData($api_call, \%sub_params); 

    foreach my $visit_group (@$data) { 

     # ignore product pages 
     next if ($visit_group->{'url'} && $visit_group->{'url'} =~ /\/products?\//); 

     # if ($visit_group->{'url'} && $visit_group->{'url'} =~ /inthenews|pressreleases|downloads/) { 
     if ($visit_group->{'idsubdatatable'}) { 
      &showcaseViewsSubData($api_call, $stat_section, $idsite, $prev_date, $last_of_month, $params, $visit_group); 
      next; 
     } 

     my $division_name; 

     if ($visit_group->{'url'}) { 
      my $showcase_id = $visit_group->{'url'}; 
      $showcase_id =~ s/^.*?\/(\d+)\/.*$/$1/; 

      $division_by_showcase_id_sth->execute($showcase_id); 
      ($division_name) = $division_by_showcase_id_sth->fetchrow_array(); 

     } else { 
      $visit_group->{'orig_label'} = $visit_group->{'label'}; 
      $visit_group->{'label'} =~ s/-/%/g; 
      $visit_group->{'label'} =~ s|^/||g; 
      $visit_group->{'label'} .= '%'; 
      $division_sth->execute($visit_group->{'label'}); 
      ($division_name) = $division_sth->fetchrow_array(); 
     } 

     if ($division_name) { 

      ## no idea why this is nb_hits, and not nb_actions, like every other method 
      my @data_value = ({ 'nb_actions' => ($visit_group->{'nb_hits'} || $visit_group->{'nb_visits'}), 'label' => $division_name }); 
      &updateCompanyStats($idsite, 'showcase', $prev_date, \@data_value); 
     } 
    } 
    return 1; 
} 

和

sub researchViewsSubData { 
    my ($api_call, $stat_section, $idsite, $prev_date, $last_of_month, $params, $subtable) = @_; 

    return unless ($subtable); 

    my %sub_params = %{ clone ($params) }; 
    $sub_params{'idSubtable'} = $subtable->{'idsubdatatable'}; 

    # $data contains views for each primary showcase page 
    my $data = &fetchStatsData($api_call, \%sub_params); 

    if (ref $data ne 'ARRAY') { 
     carp "$api_call returned something not an array!"; 
     return 0; 
    } 

    foreach my $visit_group (@$data) { 
     # if ($visit_group->{'url'} && $visit_group->{'url'} =~ /inthenews|pressreleases|downloads/) { 
     if ($visit_group->{'idsubdatatable'}) { 
      &researchViewsSubData($api_call, $stat_section, $idsite, $prev_date, $last_of_month, $params, $visit_group); 
      next; 
     } 

     my $division_name; 

     if ($visit_group->{'url'}) { 
      my $tag_id = $visit_group->{'url'}; 
      $tag_id =~ s/^.*?\/(\d+)\/.*$/$1/; 

      next if ($tag_id !~ /^\d+$/); 

      $division_by_tag_id_sth->execute(int($tag_id), int($idsite)); 
      ($division_name) = $division_by_tag_id_sth->fetchrow_array(); 

     } else { 
      carp Dumper($visit_group); 
     } 

     if ($division_name) { 
      ## no idea why this is nb_hits, and not nb_actions, like every other method 
      my @data_value = ({ 'nb_actions' => ($visit_group->{'nb_hits'} || $visit_group->{'nb_visits'}), 'label' => $division_name }); 

      &updateCompanyStats($idsite, 'research', $prev_date, \@data_value); 
     } 
    } 
    return 1; 
} 

Answer 1

至於什麼思南說，你應該從頭開始，設計一個新的類或模塊（或一組人），用於處理你所需要的功能。

下面是我根據你的代碼生成的東西。它只是給你一個方向的概要。

package MyCompanyStats; 

sub process_stats { 
    my $params = shift; 
    my $data = fetchStatsData($params); 
    foreach my $visit_group (@$data) { 
     process_stats_group($visit_group); 
    } 
} 

sub process_stats_group { 
    my $group = shift; 
    if ($group->{'url'}) { 
     my $showcase_id = get_showcase_id($group); 
     save_by_showcase_id($showcase_id, $group); 
    } else { 
     my $group_label = get_group_label($group); 
     save_by_label($group_label, $group); 
    } 
} 

sub get_group_label { 
    my $group = shift; 
    my $label = $group->{'label'}; 
    for ($label) { 
     s/-/%/g; 
     s|^/||g; 
    } 
    $label .= '%'; 
    return $label; 
} 

sub get_showcase_id { 
    my $group = shift; 
    my $showcase_id = $visit_group->{'url'}; 
    $showcase_id =~ s/^.*?\/(\d+)\/.*$/$1/; 
    return $showcase_id; 
} 

1; 

Answer 2

當你攜帶這個許多的行李，那就是你需要一些對象一個很好的案例。簡而言之，你可能把它全部包裝成一個「包」散列或使用命名對參數 - 至少它會看起來更好。

從代碼我不能告訴你爲什麼要打電話給每一個，所以我已經把笨重放入名爲theDifference的散列。

use Params::Util qw<_ARRAY _HASH>; 

sub viewsSubData { 
    my %params = @_ % 2 ? %{ &_HASH } : @_; 

    # we delete because 1. we don't pass it, and we use it once. 
    return unless my $subtable = delete $params{subtable}; 

    # If we only want a hashref to pass to fetchStatsData then 
    # stream params and the desired value in a hashref, and we're done. 
    # don't need the clone() call because listing out the hash takes care of that. 
    # $data contains views for each primary showcase page 
    my $data 
     = fetchStatsData( 
      $params{api_call} 
     , { %{ $params{params} } 
      , idSubtable => $subtable->{'idsubdatatable'} 
      } 
     ); 

    # This was made standard--because the loop will fail with the derefence, anyway 
    if (_ARRAY($data)) { 
     # returning undef is for bad states is standard in Perl 
     carp("$api_call returned something not an array!") and return; 
    } 

    my $is_showcase = $params{theDifference}; 

    foreach my $visit_group (@$data) { 

     # ignore product pages 
     next if $is_showcase 
      and $visit_group->{'url'} 
      and $visit_group->{'url'} =~ /\/products?\// 
      ; 

     # if ($visit_group->{'url'} && $visit_group->{'url'} =~ /inthenews|pressreleases|downloads/) { 
     if ($visit_group->{'idsubdatatable'}) { 
      showcaseViewsSubData(%params, subtable => $visit_group); 
      next; 
     } 

     my $division_name; 

     if ($visit_group->{'url'}) { 
      my ($tag_id) = $visit_group->{'url'}=~ m{/(\d+)/}; 

      $division_by_tag_id_sth->execute($tag_id, ($is_showcase ?() : int($params{idsite})); 
      ($division_name) = $division_by_tag_id_sth->fetchrow_array(); 

     } 
     elsif ($is_showcase) { 
      # orig_label seems to do nothing 
      for ($visit_group->{label}) { 
       s|^/||; 
       s/-/%/g; 
      } 
      $division_sth->execute($visit_group->{'label'} . '%'); 
      ($division_name) = $division_sth->fetchrow_array(); 
     } 
     else { 
      carp Dumper($visit_group) . "\n "; 
     } 

     if ($division_name) { 

      ## no idea why this is nb_hits, and not nb_actions, like every other method 
      my @data_value 
       = { nb_actions => ($visit_group->{'nb_hits'} || $visit_group->{'nb_visits'}) 
        , label  => $division_name 
        }; 
      updateCompanyStats($idsite, @params{ qw<theDifference prev_date> }, \@data_value); 
     } 
    } 
    return 1; 
} 

而你也這樣稱呼它：

viewsSubData(
    { theDifference => $whatever ? 'showcase' : 'research' 
    , api_call  => $api_call 
    , idsite  => $idsite 
    , prev_date  => $prev_date 
    , params  => $params 
    , subtable  => $subtable 
    # neither of these were used. 
    #, last_of_month => ?? 
    #, stat_section => ?? 
    });