重构现有Perl Sub

正如Sinan所说,您应该从头开始,设计一个新的类或模块(或它们的集合)来处理您需要的功能。

下面是我根据您的代码生成的内容。它的意思只是给你一个方向的轮廓。

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;

当你带着 这

从代码中,我不知道你为什么要调用每一个,所以我在名为 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  => ??
    });