Code review #2
d_x умотал в Лас-Вегас просаживать годовой бюджет маленькой африканской страны, а значит, за неимением лучших тем (точнее они есть, но я лентяй), пришло время для очередного ревью. Кстати, товарищи, неужели никто не пишет на C/C++/Asm? В запросах сплошной Perl.
Начнем с кода скрипта, который прислал Alexey Shrub. Скрипт представляет из себя сокращатель ссылок и большей частью базируется на готовых модулях. Исходный код полностью. Подход нормальный, но ревьюить толком нечего ввиду отсутствия опыта использования данных модулей. Поэтому упомяну только несколько доступных мне мелочей:
my$index=q{<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN""http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"><html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en"><head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8"/><title>URL shotener</title></head><body><form action="/"><fieldset><label for="url">Enter URL:</label><input type="text" name="url" id="url"/><input type="submit" value="Get short link"/></fieldset></form></body></html>}; |
Мне кажется, стоило бы использовать существующий heredoc-синтаксис, который позволяет избежать потенциальных проблем при модификации подобных строковых переменных, или можно было этот фрагмент вынести в отдельный файл.
my$respond=shift;my$url=$query->{url};my$url_hash= Digest::Tiger::hexhash($url);unless($url){my$w=$respond->([200,['Content-Type'=>'text/html; charset=utf-8']]);$w->write($index);undef$w;return;} |
Тут мне не особо понятна логика. Окей, мы получили переменную $url, потом попытались получить хэш от неё (ну, в php вроде бы md5 от пустой строки успешно вычисляется, так что тут наверное тоже ошибки не возникнет), а далее решили провести её валидацию. Может, стоило сначала провести валидацию, а потом уже делать всё остальное? Также здесь и далее по коду используется вызов undef $w;. Тоже непонятен смысл, переменная локальная, при выходе из зоны видимости должна уничтожаться, хотя, может, это хитрое требование используемого модуля.
И последний фрагмент из этого скрипта:
my$respond=shift;my$key=($req->path=~m!/(.+)$!)[0];$redis->get($key,{ on_done =>sub{my($url)=@_;my$w=$respond->([301,['Location'=>$url]]);$w->write("Go to $url");undef$w;} |
Меня интересует вторая строчка этого фрагмента. Я бы написал нечто вроде
my($key)=($req->path=~/(.+)$/); |
Допустим, m использован ради того, чтобы в качестве обрамляющих символов поставить восклицательный знак (но зачем?), но целесообразность использования в данном случае [0] для меня так и осталась загадкой.
Следующий скрипт нам прислал некто Abironka. Полный листинг можно посмотреть тут. Это простая отправлялка вывода команды или тела файла на pastebin.
В начале скрипта мы наблюдаем краткое описание и пример использования. Это можно было бы переоформить в соответствии со стандартом, описанным здесь. По сути неважно, но приятно, когда соблюдаются общепринятые стандарты.
our$VERSION='0.9';...if($options{h}){ usage($VERSION);exit;} |
Объявили глобальную переменную и передали её аргументом в функцию. Почему бы не обращаться к ней напрямую из функции usage? Разве что планировалось включить несколько вариантов мануалов для разных версий, но, думается, вряд ли.
if($options{i}&&!$options{s}){ simple_sintax_detect(\%options);} |
Не ради кода, но правильность написания слов стоит проверять хотя бы с помощью Google. Существуют syntax и sin tax. Думаю, речь шла о syntax.
sub simple_sintax_detect {my$options=shift; my$f_ext=$1if$options->{i}=~/^[\wа-я-]+\.([a-z]{1,4})$/i; if($f_ext=~/^(?:txt|text|)$/i){$options->{s}='text';}elsif($f_ext=~/^(?:pl|cgi)$/i){$options->{s}='perl';}elsif($f_ext=~/^sh$/i){$options->{s}='bash';}elsif($f_ext=~/^php$/i){$options->{s}='php';}} |
Вместо букв "а", "я" стоило бы использовать их hex представление, чтобы избежать возможных проблем с кодировкой при правке скрипта сторонним лицом или на специфических системах. В [\wа-я-] последнюю тирешечку стоит экранировать, а список соответствий расширений файлов языкам лучше поместить в отдельный ассоциативный массив где-нибудь в начале и таким образом избавиться от кучи elsif.
sub read_input_file {my$options=shift; if(!$options->{i}){while(<>){$msg_body.=$_;}} elsif($options->{i}){open(INFILE,'<',$options->{i})ordie"Can't open $options->{i}: $!\n";while(<INFILE>){$msg_body.=$_;}close(INFILE);} return$msg_body;} |
Тут мы виртуозно оперируем с глобальной переменной $msg_body, а потом её же и возвращаем зачем-то. Наверное, стоило передать её внутрь, либо объявить как локальную внутри, либо не передавать, но и не писать return $msg_body. Кстати, второй вариант чтения файла можно было реализовать более идиоматически (при условии, что мы используем вариант с return ...). Примерно так:
sub read_file {local@ARGV=shift;local$/=wantarray?$/:undef;<>;} |
Естественно, с адаптацией под данный случай. И последний фрагмент:
sub ua_init { my$cookies=HTTP::Cookies->new('file'=>'./cookies.lwp','autosave'=>0); my$ua= LWP::UserAgent->new('agent'=>'pastebinput - pastebin service agent; Этот e-mail адрес защищен от спам-ботов, для его просмотра у Вас должен быть включен Javascript ','cookie_jar'=>$cookies,'requests_redirectable'=>['GET','POST'],); $ua->default_header('Accept'=>'text/html, application/xml;q=0.9, application/xhtml+xml, */*;q=0.1','Accept-Charset'=>'utf-8; *;q=0.1','Accept-Language'=>'ru,en-us;q=0.7,en;q=0.3','Accept-Encoding'=>'deflate, gzip, x-gzip, *;q=0',); return$ua;} |
Создаем объект HTTP::Cookies, хорошо, но файл для хранения указывать не имеет смысла, на мой взгляд. Пусть в памяти хранит. Далее мы передаем созданный объект в качестве значения cookie_jar для LWP::UserAgent и в дальнейшем переменную $cookies не используем. Тогда можно было бы ограничиться следующим кодом:
my$ua= LWP::UserAgent->new('agent'=>'pastebinput - pastebin service agent; Этот e-mail адрес защищен от спам-ботов, для его просмотра у Вас должен быть включен Javascript ','cookie_jar'=>new HTTP::Cookies,'requests_redirectable'=>['GET','POST'],); |
Последний на сегодня скрипт, а точнее модуль, присланный e.s. С листингом можно ознакомиться здесь. Этот модуль предназначен для получения доступной информации по сайту из Яндекс.Каталога (название, описание, тИЦ, рубрики...). Кода в нем мало, документация оформлена надлежащим образом, поэтому рассмотрим единственную более-менее крупную функцию - yaca_lookup, которая реализует основной функционал данного модуля. Рассматривать будет фрагментами.
$address=~s|.*?://||;# loose scheme$address=~s|.*?(:.*?)?@||;# loose authentication$address=~s|(\w):\d+|$1|;# loose port$address=~s|\?.*||;# loose query$address=~s|/$||;# loose trailing slash my$contents= get('http://search.yaca.yandex.ru/yca/cy/ch/'.$address); |
Мы получили на вход URL и хотим получить информацию о нём из Яндекс.Каталога. В данном фрагменте адрес приводится к некому каноничному для Яндекса виду. Я бы посоветовал не играться с регулярными выражениями в данном случае, а воспользоваться модулем URI::Split, тем более он входит в стандартный комплект модулей (по крайней мере, в случае с ActivePerl).
if($contents=~/<p class="b-cy_error-cy">/){# "ресурс не описан в Яндекс.Каталоге"# It's not in the catalog, but tIC is always displayed.# Ex.: Индекс цитирования (тИЦ) ресурса — 10($self->{_tic})=$contents=~/<p class="b-cy_error-cy">.*?\s(\d+)/s;$self->{_tic}=0unlessdefined$self->{_tic};} |
Последние несколько строк можно было записать проще, например, следующим образом:
$self->{_tic}=$contents=~/<p class="b-cy_error-cy">.*?\s(\d+)/s?$1:0; |
Далее
my($entry)=$contents=~qr{(<tr>\s*<td><img.*/arr-hilite\.gif".*?</tr>)}s; ( $self->{_orderNum}, $self->{_uri}, $self->{_shortDescr}, undef, $self->{_longDescr}, $self->{_tic} ) = # $1 $2 $3 $4 $5 $entry =~ qr{<td>(\d+)\.\s*</td>.*<a href="/(.*?)".*?>(.*)</a>(<div>(.*)</div>.*?)?(\d+)<}s; |
Исходя из undef в списке переменных, делаю предположение, что автор не в курсе non-capturing groupings, которым можно воспользоваться для отсечения захвата ненужных результатов.
if($path){$path=~s{</?a.*?>|</?h1>|\n}{}gs;# remove A, H1 tags and newline$path=~s|\x{0420}\x{0443}\x{0431}\x{0440}\x{0438}\x{043A}\x{0438}/||;# removed "Рубрики" - it always starts with it# http://www.rishida.net/tools/conversion/push(@{$self->{_categories}},$path.' / '.$rubric)if$entry;} |
Регулярные выражения для удаления HTML-разметки? А если новые возможные элементы добавят? Можно было воспользоваться HTML::Strip. Оставшийся код особых нареканий не вызвал.
На этом, пожалуй, завершу обзор. Если ваш код не попал в очередное ревью - не волнуйтесь, всё, что приходит, рано или поздно будет разобрано.
Также рекомендую почитать
Обсудить на форуме
Источник: http://feedproxy.google.com/~r/kaimi/dev/~3/H6UyAPX6Q3g/
|
d_x умотал в Лас-Вегас просаживать годовой бюджет маленькой африканской страны, а значит, за неимением лучших тем (точнее они есть, но я лентяй), пришло время для очередного ревью. Кстати, товарищи, |
РэдЛайн, создание сайта, заказать сайт, разработка сайтов, реклама в Интернете, продвижение, маркетинговые исследования, дизайн студия, веб дизайн, раскрутка сайта, создать сайт компании, сделать сайт, создание сайтов, изготовление сайта, обслуживание сайтов, изготовление сайтов, заказать интернет сайт, создать сайт, изготовить сайт, разработка сайта, web студия, создание веб сайта, поддержка сайта, сайт на заказ, сопровождение сайта, дизайн сайта, сайт под ключ, заказ сайта, реклама сайта, хостинг, регистрация доменов, хабаровск, краснодар, москва, комсомольск |
Дайджест новых статей по интернет-маркетингу на ваш email
Новые статьи и публикации
- 2026-03-04 » Скорость, ИИ и человекоцентричность: каким должен быть сайт в 2026 году
- 2026-03-03 » Как искусственный интеллект меняет таргетированную рекламу
- 2026-03-03 » Основные киберугрозы для бизнеса: 5 способов потерять (и сохранить) данные компании
- 2026-03-03 » Главные тренды веб-дизайна 2026 года: от гиперминимализма до кибербрутализма
- 2026-03-03 » Искусственный интеллект в маркетинге: помощник, а не замена человеку
- 2026-02-26 » Нулевая позиция в поиске: как на нее попасть и зачем это нужно
- 2026-02-26 » Как выбрать подрядчика для сайта и не попасть на мошенников
- 2026-02-26 » Инклюзивный клиентский опыт: как сделать бизнес доступным для всех и повысить конверсию
- 2026-02-26 » ESG-принципы: что это такое и как бизнесу внедрять их в свою работу
- 2026-02-26 » Почему в 2026 году маркетологу не стоит бояться искусственного интеллекта
- 2026-02-22 » No-code vs Профессиональная разработка: выстрелит ли Tilda в 2026 году?
- 2026-02-22 » Почему малый бизнес проигрывает в контекстной рекламе и при чем тут структура сайта
- 2026-02-22 » Куда уходит скорость: как мы теряем посетителей из-за одного "тяжелого" шрифта
- 2026-02-22 » Дарк-паттерны (Dark Patterns) в интерфейсах: когда манипуляция клиентом выходит боком
- 2026-02-22 » Голосовой поиск и веб: готов ли ваш сайт к разговору с Алисой и Марусей?
- 2026-02-22 » Микроанимация и UX/UI: как движение элементов влияет на конверсию
- 2026-02-22 » ИИ в веб-аналитике: как нейросети предсказывают отток клиентов до того, как они уйдут
- 2026-02-22 » Темная сторона шаблонов: почему сайт на готовом решении может угробить ваш бизнес
- 2026-02-22 » Зеленый хостинг и экология в IT: тренд или необходимость?
- 2026-02-22 » Веб-доступность (Accessibility): почему ваш сайт теряет до 20% клиентов и штрафует сам себя
- 2026-02-12 » Экономика фриланса vs веб-студии: скрытые издержки и риски при заказе сайта «у знакомого разработчика»
- 2026-02-12 » Инструменты аналитики помимо Google Analytics
- 2026-02-12 » Юридические аспекты владения сайтом
- 2026-02-12 » Сравниваем популярные CRM-системы для интеграции с сайтом
- 2026-02-12 » Эволюция интерфейсов: от CLI к GUI, к VUI и далее
- 2026-02-12 » Контент-стратегия после обновления Google Helpful Content
- 2026-02-12 » Headless-архитектура: модный тренд или необходимость для вашего бизнеса?
- 2026-02-12 » Мифы о кибербезопасности для малого и среднего бизнеса
- 2026-02-12 » Как Core Web Vitals влияют не только на SEO, но и на конверсию?
- 2026-02-12 » PWA vs Нативное приложение: что выбрать малому бизнесу в 2026?
Дураки ставят вопросы чаще, чем пытливые люди Горький Максим - (1868-1936) - русский писатель, литературный критик и публицист, общественный деятель |
Мы создаем сайты, которые работают! Профессионально обслуживаем и продвигаем их !
Как мы работаем
Заявка
Позвоните или оставьте заявку на сайте.
Консультация
Обсуждаем что именно Вам нужно и помогаем определить как это лучше сделать!
Договор
Заключаем договор на оказание услуг, в котором прописаны условия и обязанности обеих сторон.
Выполнение работ
Непосредственно оказание требующихся услуг и работ по вашему заданию.
Поддержка
Сдача выполненых работ, последующие корректировки и поддержка при необходимости.
Мы создаем практически любые сайты от продающих страниц до сложных, высоконагруженных и нестандартных веб приложений! Наши сайты это надежные маркетинговые инструменты для успеха Вашего бизнеса и увеличения вашей прибыли! Мы делаем красивые и максимально эффектные сайты по доступным ценам уже много лет!
Комплексный подход это не просто продвижение сайта, это целый комплекс мероприятий, который определяется целями и задачами поставленными перед сайтом и организацией, которая за этим стоит. Время однобоких методов в продвижении сайтов уже прошло, конкуренция слишком высока, чтобы была возможность расслабиться и получать \ удерживать клиентов из Интернета, просто сделав сайт и не занимаясь им...
Мы оказываем полный комплекс услуг по сопровождению сайта: информационному и техническому обслуживанию и развитию Интернет сайтов.
Контекстная реклама - это эффективный инструмент в интернет маркетинге, целью которого является увеличение продаж. Главный плюс контекстной рекламы заключается в том, что она работает избирательно.