Информация об изменениях

Сообщение Re[5]: Ой ли? от 06.11.2018 22:39

Изменено 06.11.2018 22:40 Artem Korneev

Re[5]: Ой ли?
Здравствуйте, Sheridan, Вы писали:

Б>>Perl — write only язык, когда используются его возможности из "Practical Extraction and Report Language"

S> Держи нопример и меняй мнение.

В целом — лучше, чем в среднем по больнице, но мнение о читабельности перла этот исходник едва ли изменит.

sub what_item_at_coordinates
{
  [...]
  opendir($dir_h, $items_dir) or die $!;
  while (my $item = readdir($dir_h))


Перемешивание IO-операций с бизнес-логикой не есть хорошо.

Еще бросается в глаза многократные копи-пасты одних и тех же выражений:

-d $items_dir.$item &&
(
  (exists($self->{'items_to_find'}{$item}) || scalar(keys(%{$self->{'items_to_find'}})) == 1) ||
  (exists($self->{'items_to_find'}{'any-plank'}) && $item=~/-plank/) ||
  (exists($self->{'items_to_find'}{'any-wood-slab'}) && $item=~/-slab/) ||
  (exists($self->{'items_to_find'}{'any-stone-brick'}) && $item=~/([^d]s|cracked-s)tone-brick-block/)
)


Даже если довериться компилятору и считать, что результат вычисления $self->{'items_to_find'} закэшируется и не будет вычислен заново 5 раз подряд в этом if, то все равно, для читабельности лучше было бы сохранить результат в какой-либо именованной переменной.

И сложные вложенные конструкции:

$main::player->hand()->put_one_item_to_cell($main::config->{'system'}{$self->{'interface'}}{$self->{'interface_target'}}{$x}{$y});


Оно как бы не упрощает читабельность. Я тут сходу глазами не могу распарсить, какая скобка куда относится и какой параметр куда попадает. Есть хорошее правило: одна строка — одно действие.
Re[5]: Ой ли?
Здравствуйте, Sheridan, Вы писали:

Б>>Perl — write only язык, когда используются его возможности из "Practical Extraction and Report Language"

S> Держи нопример и меняй мнение.

В целом — лучше, чем в среднем по больнице, но мнение о читабельности перла этот исходник едва ли изменит.

sub what_item_at_coordinates
{
  [...]
  opendir($dir_h, $items_dir) or die $!;
  while (my $item = readdir($dir_h))


Перемешивание IO-операций с бизнес-логикой не есть хорошо.

Еще бросаются в глаза многократные копи-пасты одних и тех же выражений:

-d $items_dir.$item &&
(
  (exists($self->{'items_to_find'}{$item}) || scalar(keys(%{$self->{'items_to_find'}})) == 1) ||
  (exists($self->{'items_to_find'}{'any-plank'}) && $item=~/-plank/) ||
  (exists($self->{'items_to_find'}{'any-wood-slab'}) && $item=~/-slab/) ||
  (exists($self->{'items_to_find'}{'any-stone-brick'}) && $item=~/([^d]s|cracked-s)tone-brick-block/)
)


Даже если довериться компилятору и считать, что результат вычисления $self->{'items_to_find'} закэшируется и не будет вычислен заново 5 раз подряд в этом if, то все равно, для читабельности лучше было бы сохранить результат в какой-либо именованной переменной.

И сложные вложенные конструкции:

$main::player->hand()->put_one_item_to_cell($main::config->{'system'}{$self->{'interface'}}{$self->{'interface_target'}}{$x}{$y});


Оно как бы не упрощает читабельность. Я тут сходу глазами не могу распарсить, какая скобка куда относится и какой параметр куда попадает. Есть хорошее правило: одна строка — одно действие.