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
Новые статьи и публикации
- 2025-07-09 » Индексация сайта: что это и как ускорить попадание страниц в поисковики
- 2025-07-09 » 4 маркетинговых трюка от Стива Джобса
- 2025-07-08 » Как мышление влияет на успех бизнеса, и причем тут эмоциональный интеллект
- 2025-07-08 » «Бомж-маркетинг» без бюджета: механика, тактики и результаты
- 2025-07-08 » Медицинский интернет-маркетинг: каналы, особенности и рекомендации
- 2025-07-04 » Утечка данных стоит дороже, чем их защита: почему вам нужна грамотная ИТ-инфраструктура
- 2025-07-04 » Персональные данные: правила хранения и обработки и изменения 2025 года
- 2025-07-04 » Владельцам сайтов: изменения в законе о персональных данных
- 2025-07-04 » Персональные данные: самый полный гайд на 2025 год
- 2025-07-04 » Штрафы за нарушения в работе с персональными данными с 30 мая 2025 года: утечка в интернет и неуведомление РКН
- 2025-07-04 » Новые штрафы в работе с персональными данными: что проверить прямо сейчас
- 2025-07-04 » Google выкатил 68-страничный гайд по промптам. Я прочитал его за вас и вытащил 4 главных правила
- 2025-07-04 » Как выстроить доверие и лояльность клиентов через точки контакта
- 2025-07-04 » Пять SEO-правил, которые реально работают в 2025 году
- 2025-06-10 » Кому нужно срочно подать уведомление в РКН об обработке персональных данных и как это правильно сделать
- 2025-06-10 » Что такое VPN и зачем он нужен?
- 2025-06-10 » Нейросети для создания видео: 7 инструментов и что они могут
- 2025-06-10 » ChatGPT, DeepSeek, Grok, Gemini доступны на русском бесплатно. Внедряем?
- 2025-06-10 » 12 нейросетей для работы с маркетплейсами: создание карточек и описаний для Wildberries и Ozon
- 2025-06-10 » 11 нейросетей для генерации изображений в 2025 году
- 2025-05-30 » Год назад то, что занимало у меня несколько дней работы, сейчас я делаю за 1-2 часа. Без преувеличений. И это только начало
- 2025-05-25 » Нейросети для написания текста: 7 сервисов в помощь копирайтеру
- 2025-05-25 » Сайты с качественным контентом смогут получать больше трафика после обновления алгоритмов в Поиске Яндекса
- 2025-05-07 » Почему страницы не индексируются Google: три типа проблем
- 2025-05-05 » Лидеры рейтинга самых дорогих компаний Рунета — 2025
- 2025-05-05 » Мы делали презентации 35 лет, а потом пришла нейросеть
- 2025-04-08 » Горшочек, рисуй: 10 бесплатных сервисов для генерации картинок
- 2025-04-08 » SEO-продвижение в 2025 году: 15 трендов, без которых ТОП не светит
- 2025-03-14 » SPF-запись
- 2025-03-07 » SEO на маркетплейсах: как оптимизировать карточку товара для поисковой выдачи
Одного яйца два раза не высидишь. К. Прутков |
Мы создаем сайты, которые работают! Профессионально обслуживаем и продвигаем их !
Как мы работаем
Заявка
Позвоните или оставьте заявку на сайте.
Консультация
Обсуждаем что именно Вам нужно и помогаем определить как это лучше сделать!
Договор
Заключаем договор на оказание услуг, в котором прописаны условия и обязанности обеих сторон.
Выполнение работ
Непосредственно оказание требующихся услуг и работ по вашему заданию.
Поддержка
Сдача выполненых работ, последующие корректировки и поддержка при необходимости.
Мы создаем практически любые сайты от продающих страниц до сложных, высоконагруженных и нестандартных веб приложений! Наши сайты это надежные маркетинговые инструменты для успеха Вашего бизнеса и увеличения вашей прибыли! Мы делаем красивые и максимально эффектные сайты по доступным ценам уже много лет!
Комплексный подход это не просто продвижение сайта, это целый комплекс мероприятий, который определяется целями и задачами поставленными перед сайтом и организацией, которая за этим стоит. Время однобоких методов в продвижении сайтов уже прошло, конкуренция слишком высока, чтобы была возможность расслабиться и получать \ удерживать клиентов из Интернета, просто сделав сайт и не занимаясь им...
Мы оказываем полный комплекс услуг по сопровождению сайта: информационному и техническому обслуживанию и развитию Интернет сайтов.
Контекстная реклама - это эффективный инструмент в интернет маркетинге, целью которого является увеличение продаж. Главный плюс контекстной рекламы заключается в том, что она работает избирательно.