Почему Perl :: Critic не любит использовать shift для заполнения переменных подпрограммы?

Question

В последнее время я решил начать использовать Perl :: Critic чаще в своем коде. После программирования на Perl в течение почти 7 лет я уже давно освоился с большинством лучших практик Perl, но я знаю, что всегда есть возможности для улучшений. Однако меня беспокоит то, что Perl :: Critic не нравится то, как я распаковываю @_ для подпрограмм. Как пример:

sub my_way_to_unpack {
    my $variable1 = shift @_;
    my $variable2 = shift @_;

    my $result = $variable1 + $variable2;
    return $result;
}

Так я всегда делал, и, как обсуждалось на PerlMonks и Stack Overflow, его не обязательно злой .

Изменение приведенного выше фрагмента кода на ...

sub perl_critics_way_to_unpack {
    my ($variable1, $variable2) = @_;

    my $result = $variable1 + $variable2;
    return $result;
}

... тоже работает, но мне труднее читать. Я также прочитал книгу Дамиана Конвея Perl Best Practices , и я не совсем понимаю, как мой предпочтительный подход к распаковке подпадает под его предложение избегать использования @_ напрямую, как Perl :: Critic подразумевает. У меня всегда было впечатление, что Конвей говорил о таких мерзостях, как:

sub not_unpacking {
    my $result = $_[0] + $_[1];
    return $result;
}

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

Короче говоря, почему Perl :: Critic считает мой предпочтительный путь плохим? Я действительно совершаю чудовищное преступление, распаковывая с помощью смены?

Может ли это быть чем-то, что другие люди, кроме меня, считают воспитателем Perl :: Critic ?

Answer 1

Простой ответ заключается в том, что Perl :: Critic здесь не следует PBP. Книга недвусмысленно гласит, что идиома сдвига не только приемлема, но и на самом деле является предпочтительным в некоторых случаях.

Answer 2

Запуск perlcritic с --verbose 11 объясняет политику. Похоже, что ни одно из этих объяснений не относится к вам.

Always unpack @_ first at line 1, near 
'sub xxx{ my $aaa= shift; my ($bbb,$ccc) = @_;}'.
  Subroutines::RequireArgUnpacking (Severity: 4)
    Subroutines that use `@_' directly instead of unpacking the arguments to
    local variables first have two major problems. First, they are very hard
    to read. If you're going to refer to your variables by number instead of
    by name, you may as well be writing assembler code! Second, `@_'
    contains aliases to the original variables! If you modify the contents
    of a `@_' entry, then you are modifying the variable outside of your
    subroutine. For example:

       sub print_local_var_plus_one {
           my ($var) = @_;
           print ++$var;
       }
       sub print_var_plus_one {
           print ++$_[0];
       }

       my $x = 2;
       print_local_var_plus_one($x); # prints "3", $x is still 2
       print_var_plus_one($x);       # prints "3", $x is now 3 !
       print $x;                     # prints "3"

    This is spooky action-at-a-distance and is very hard to debug if it's
    not intentional and well-documented (like `chop' or `chomp').

    An exception is made for the usual delegation idiom
    `$object->SUPER::something( @_ )'. Only `SUPER::' and `NEXT::' are
    recognized (though this is configurable) and the argument list for the
    delegate must consist only of `( @_ )'.

Answer 3

Важно помнить, что многие вещи в Perl Best Practices - это просто мнение одного парня о том, что выглядит лучше или с которым легче всего работать, и не имеет значения, если вы это делаете это по-другому. Дамиан говорит так же во вступительном тексте к книге. Нельзя сказать, что это все , как это - есть много вещей, которые абсолютно необходимы: например, использование strict.

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

Я пытаюсь следовать большинству вещей в PBP, но у Дамиана могут быть мои аргументы подпрограммы shift s и мои unless es, когда он вытаскивает их из моих холодных, мертвых пальцев.

Что касается Critic, вы можете выбрать, какие политики вы хотите применить, и даже создать свои собственные, если они еще не существуют.

Answer 4

В некоторых случаях Perl :: Critic не может точно применять правила PBP, поэтому он может применять приближение, которое пытается соответствовать духу рекомендаций Conway. И вполне возможно, что мы неправильно истолковали или неправильно применили PBP. Если вы найдете что-то, что не пахнет правильно, отправьте сообщение об ошибке на bug-perl-critic@rt.cpan.org, и мы сразу же рассмотрим это.

Спасибо

-Джефф

Answer 5

Я думаю, что вы вообще должны избегать сдвига, если в этом нет особой необходимости!

Просто наткнулся на такой код:

sub way {
  my $file = shift;
  if (!$file) {
    $file = 'newfile';
  }
  my $target = shift;
  my $options = shift;
}

Если вы начнете что-то менять в этом коде, есть большая вероятность, что вы можете случайно изменить порядок смен или, возможно, пропустить один, и все пойдет на юг. Кроме того, его трудно читать - потому что вы не можете быть уверены, что действительно видите все параметры для подпрограммы, потому что некоторые строки ниже могут быть другим сдвигом где-то ... И если вы используете несколько регулярных выражений между ними, они могут заменить содержимое $ _ и странные вещи начинают происходить ...

Прямая выгода от использования распаковки my (...) = @_ заключается в том, что вы можете просто скопировать (...) часть и вставить ее туда, где вы вызываете метод, и иметь хорошую подпись :), которую вы даже можете использовать заранее одни и те же имена переменных и не нужно ничего менять!

Я думаю, что shift подразумевает операции со списком, когда длина списка является динамической, и вы хотите обрабатывать его элементы по одному или когда вам явно нужен список без первого элемента. Но если вы просто хотите назначить весь список x параметрам, ваш код должен сказать это с помощью my (...) = @_; никто не должен удивляться.