Хабр Курсы для всех
РЕКЛАМА
Практикум, Хекслет, SkyPro, авторские курсы — собрали всех и попросили скидки. Осталось выбрать!
Хабр доступен 24/7 благодаря поддержке друзей
Комментарии 45
За такой код денег вам точно никто не даст, а статья чуть менее чем полностью состоит из не очень качественного кода, рекламы сайта своей компании и попрошайничества.
PS научитесь хотябы автолоадом пользоваться для начала :)
PS научитесь хотябы автолоадом пользоваться для начала :)
$operation_id = $_POST['operation_id'];
$sender = $_POST['sender'];
$amount = $_POST['amount'];
$datetime = $_POST['datetime'];
preg_match('/i(\d+);/', $resp->getMessage(), $m);
$invoice_id = $m[1];
$r=mysql_query("INSERT INTO it_payment_ym VALUES('$operation_id', '$sender', '$amount', '$datetime', '$invoice_id')");
Вы серьезно значения, полученные из $_POST, подставляете непосредственно в SQL запрос?
В данном случае, этим значениям можно доверять, т.к. выше была проверка
if(sha1($str)!=$_POST['sha1_hash'])
return mail('igor@itsoft.ru', 'Fake notification', $message);
И был запрос к Яндексу. Для того, чтобы эти проверки обойти нужно:
1. знать секретный код
2. каким-то образом вставить данные в БД Яндекса
Все эти значения числовые. Вряд ли Яндекс в них пришлет кавычку.
Но для полной уверенности можно их конечно и профильтровать. Хотя у нас на сервере стоит автоматическая фильтрация переменных.
if(sha1($str)!=$_POST['sha1_hash'])
return mail('igor@itsoft.ru', 'Fake notification', $message);
И был запрос к Яндексу. Для того, чтобы эти проверки обойти нужно:
1. знать секретный код
2. каким-то образом вставить данные в БД Яндекса
Все эти значения числовые. Вряд ли Яндекс в них пришлет кавычку.
Но для полной уверенности можно их конечно и профильтровать. Хотя у нас на сервере стоит автоматическая фильтрация переменных.
Уважаемый So1!
1. Мне все равно дадут мне денег или нет, просто кошелек был в коде, вот я его и поставил, на то и смайлик в конце стоит. Во всяком случае так было принято писать в 90е, когда я активно работал программистом.
2. Если есть замечания по качеству кода, то их наверно имеет высказывать, а не агрессию. Уточните, пожалуйста, какой именно автолоад имеется в виду?
3. Моя компания в рекламе не нуждается. По всем профильным коммерческим запросам первые страницы в поисковиках. Заказов на месяцы вперед.
На то чтобы разобраться со всем, что описано выше, найти документацию, нагуглить ряд вопросов, интегрировать одно с другим, отладить у меня пара дней ушло. Возможно, я уже не очень хороший PHP-программист, я не претендую ни на что. Но за час 22 человека в избранное статью добавили, значит это все-таки кому-то надо. Если бы на Яндексе была подобная статься, то на прикрутку и отладку у меня ушло бы 1-2 часа времени.
1. Мне все равно дадут мне денег или нет, просто кошелек был в коде, вот я его и поставил, на то и смайлик в конце стоит. Во всяком случае так было принято писать в 90е, когда я активно работал программистом.
2. Если есть замечания по качеству кода, то их наверно имеет высказывать, а не агрессию. Уточните, пожалуйста, какой именно автолоад имеется в виду?
3. Моя компания в рекламе не нуждается. По всем профильным коммерческим запросам первые страницы в поисковиках. Заказов на месяцы вперед.
На то чтобы разобраться со всем, что описано выше, найти документацию, нагуглить ряд вопросов, интегрировать одно с другим, отладить у меня пара дней ушло. Возможно, я уже не очень хороший PHP-программист, я не претендую ни на что. Но за час 22 человека в избранное статью добавили, значит это все-таки кому-то надо. Если бы на Яндексе была подобная статься, то на прикрутку и отладку у меня ушло бы 1-2 часа времени.
2. spl_autoload_register. Но вот нужен ли он в примерах — большой вопрос. Ручное подключение файлов позволяет наглядно показать что именно задействовано. Код у вас конечно не ахти, но если это примеры для общего ознакомления, а не части рабочей системы, то простительно.
А в чем не ахти?
Тут кода то почти нет.
Тут кода то почти нет.
начиная от переменных внутри строки, особенно $i->id в примере
отсутствия фигурных скобок в блочных конструкциях типа if-else
и до откровенно некрасивой работы с базой
src="https://money.yandex.ru/embed/shop.xml?uid=4100138353971&writer=seller&targets=i$i->id&default-sum=$sum&button-text=01&hint="
отсутствия фигурных скобок в блочных конструкциях типа if-else
и до откровенно некрасивой работы с базой
А что же такого в переменных внутри строки? Вот framework.zend.com/manual/1.12/ru/coding-standard.coding-style.html тут это не возбраняется. Не sprintf же использовать.
Как старый сионист фигурные скобки не признаю для одной строки if-else.
А как с ней красиво то работают сейчас? Каждый тут волен использовать свои классы или библиотеки. Такая работа с БД в большинстве примеров на php.net и прочей документации.
Как старый сионист фигурные скобки не признаю для одной строки if-else.
А как с ней красиво то работают сейчас? Каждый тут волен использовать свои классы или библиотеки. Такая работа с БД в большинстве примеров на php.net и прочей документации.
Как старый сионист фигурные скобки не признаю для одной строки if-else.
вот, собственно, в этом и ваша проблема. бесполезно спорить. просто поверьте — код в таком виде небрежен, нечитаем, а где-то и опасен — эту мысль вам и попытались донести.
Спорить как раз полезно. На эту тему споров было много еще в 90е.
Верят в бога, религию. А в науке надо доказывать. Так вот есть аргументация за и против, и в обоих стилях, есть свои ЗА и ПРОТИВ. Все это уже описано много раз и можно найти.
В конечном итоге, код легко транслировать в тот стиль, который Вам ближе.
Стиль — штука субъективная. Для меня более опасен стиль с кучей скобок.
Так что тут дело вкуса и предпочтений.
Верят в бога, религию. А в науке надо доказывать. Так вот есть аргументация за и против, и в обоих стилях, есть свои ЗА и ПРОТИВ. Все это уже описано много раз и можно найти.
В конечном итоге, код легко транслировать в тот стиль, который Вам ближе.
Стиль — штука субъективная. Для меня более опасен стиль с кучей скобок.
Так что тут дело вкуса и предпочтений.
НЕ вижу смысла расписывать такую мелочь, Я.Деньги в стандартном варианте подключается ровно так же как и десятки других платежных систем. Слава богу с интерфейсами подключения более-менее устаканилось и все они похожи -+ мелочи
Ох, как я рад, что вовремя соскочил с пхп! Это же ад :)
Никаких правил в именовании файлов, никакого mvc, никакого orm, все ужасно смотрится (по крайней мере тут, но это в целом тенденция пхп, извиняюсь заранее перед настоящими программистами хпх :)
Никаких правил в именовании файлов, никакого mvc, никакого orm, все ужасно смотрится (по крайней мере тут, но это в целом тенденция пхп, извиняюсь заранее перед настоящими программистами хпх :)
да, пхп развращает свободой, программист сам должен поддерживать дисциплину. проще всего следовать Zend Coding Style, использовать качественные фрэймворки и все будет хорошо :)
Предлагаю сравнить ваш стайл-гайд с рубишным :)
Разница в том, что ваш нужно соблюдать, чтобы уж совсем не погрязнуть в куче синтаксического мусора. А рубишный — чтобы код стал еще более человечным и элегантным.
Разница в том, что ваш нужно соблюдать, чтобы уж совсем не погрязнуть в куче синтаксического мусора. А рубишный — чтобы код стал еще более человечным и элегантным.
И теперь у вас стал на 3 см. длиннее?
Балиин… Что же нам делать, тем, кто подсажен злобным Расмусом на адов ПХП?
Хнык-хнык.
Btw, языки никак не связаны с правилами, паттернами и фреймворками. Слава б-гу, что в нашем адовом стане ПХП нет уже того, кто этого недопонимает.
зы: Я не против %languagename%, я за %braininthehead%.
Балиин… Что же нам делать, тем, кто подсажен злобным Расмусом на адов ПХП?
Хнык-хнык.
Btw, языки никак не связаны с правилами, паттернами и фреймворками. Слава б-гу, что в нашем адовом стане ПХП нет уже того, кто этого недопонимает.
зы: Я не против %languagename%, я за %braininthehead%.
Всё это есть, но всё это не обязательно — хочешь используй, а хочешь нет (если не боишься, что уволят). Да и в других мэйнстримовых языках почти то же самое.
А нафига Игору вечно на ящик спамить?
Для меня это самый удобный и оперативный способ узнать, если что-то пошло не так.
Сообщения отправляются только в случае возникновения каких-либо ошибок.
Каждый волен переписать под себя и складывать в логи, которые никто не читает, как правило.
Сообщения отправляются только в случае возникновения каких-либо ошибок.
Каждый волен переписать под себя и складывать в логи, которые никто не читает, как правило.
Правильно ли я понимаю, что полезный код вашего примера укладывается в(без геттеров/сеттеров, компактности ради):
?
К слову, я бы не назвал приведенный выше кусок кода хорошим. Он имеет ряд недостатков — жесткое внедрение зависимости, отсутствие проверки данных в массиви $data, отсутствие интерфейса, отсутствие исключений для обработки и многое другое. Но его хотя бы можно рефакторить.
Ваш же код это откровенная бурда из ветвлений, создания буферов вывода, отправок сообщений Игорю и записи в БД.
Вы же понимаете, что то, что если для вас этот пахучий кусочек какашки работает(у нас у всех такие бывают, чего греха таить) совершенно не означает, что сообществу(а не дай бог новичкам) он будет полезен? Вреден.
<?
require_once($_SERVER["DOCUMENT_ROOT"] . "/conf/config_db_ro_without_auth1.php");
require_once(dirname(__FILE__) . '/lib/YandexMoney.php');
require_once(dirname(__FILE__) . '/sample/consts.php');
class CProcessYandexMoney
{
private $_data = array();
private $_secretKey = null;
private $_token = null;
private $_clientId = null;
public function __construct($data, $secretKey, $token, $clientId)
{
$this->_setData($data);
$this->_setSecretKey($secretKey);
}
public function process()
{
//Application::getLogger()->logArray($this->getData());
return $this->_processPayment();
}
private function _processPayment()
{
$yandexMoneyAdapter = new YandexMoneyNew($this->getClientId());
$originData = $this->getData();
$processing = $yandexMoneyAdapter->operationDetail($this->getToken(), $originData['operation_id']);
if ($processing->isSuccess()) {
return true;
} else {
return false;
}
}
protected function _checkPaymentType()
{
$originData = $this->getData();
return $originData['codepro'] != 'false';
}
protected function _checkSignification()
{
$originData = $this->getData();
return $this->_createSignification() == $originData['sha1_hash'];
}
protected function _createSignification()
{
$originData = $this->getData();
$signData = array(
$originData['notification_type'],
$originData['operation_id'],
$originData['amount'],
$originData['currency'],
$originData['datetime'],
$originData['sender'],
$originData['codepro'],
$this->getSecretKey(),
$originData['label']
);
$signReqString = implode('&', $signData);
$sign = sha1($signReqString);
return $sign;
}
}
?
К слову, я бы не назвал приведенный выше кусок кода хорошим. Он имеет ряд недостатков — жесткое внедрение зависимости, отсутствие проверки данных в массиви $data, отсутствие интерфейса, отсутствие исключений для обработки и многое другое. Но его хотя бы можно рефакторить.
Ваш же код это откровенная бурда из ветвлений, создания буферов вывода, отправок сообщений Игорю и записи в БД.
Вы же понимаете, что то, что если для вас этот пахучий кусочек какашки работает(у нас у всех такие бывают, чего греха таить) совершенно не означает, что сообществу(а не дай бог новичкам) он будет полезен? Вреден.
Вы меня извините, но мне не интересно вести дискуссию в подобном тоне. Она не имеет смысла без элементарного взаимного уважения.
Мне кажется, что вы неадекватны.
Я потратил своё время на прочтение вашей статьи, пример более-менее вменяемой имплементации, а также на указание явных ошибок. Если вы с чем-то не согласны, давайте обсудим как программисты. Это будет как-минимум не бесполезно.
Вы же говорите не предметно о каких-то абстрактных сущностях вроде «тона» и «уважения».
Тезис 1 — вам не ведом мой тон
Тезис 2 — мне не за что вас уважать, но и неуважать тоже
Я потратил своё время на прочтение вашей статьи, пример более-менее вменяемой имплементации, а также на указание явных ошибок. Если вы с чем-то не согласны, давайте обсудим как программисты. Это будет как-минимум не бесполезно.
Вы же говорите не предметно о каких-то абстрактных сущностях вроде «тона» и «уважения».
Тезис 1 — вам не ведом мой тон
Тезис 2 — мне не за что вас уважать, но и неуважать тоже
Это очень важно. Дискутировать я согласен, но не оскорблять собеседника.
По существу Ваш код хороший пример использования ООП там, где оно не нужно. ООП имеет 4 основных и три дополнительных отличительных признака согласно Гради Бучу. Если ничего из это нет необходимости использовать, то код лучше оставить структурным.
Скрипт делает свое дело по вполне четкой схеме. Рефакторить там нечего. Протокол в нормальных системах не меняется. Там может появиться только в самом конце код связанный с исполнением заказа. Этот код может быть уже на чем угодно в зависимости от сайта и товаров, которые он продает.
Ваш же код это откровенная бурда из ветвлений
Бурда — это неправильная логика или логика, которую можно оптимизировать. Покажите, что там можно упростить.
создания буферов вывода, отправок сообщений Игорю и записи в БД.
И что Вы предлагаете вместо всего перечисленного? Мне нужно именно email в случае ошибки и именно запись в БД.
если для вас этот пахучий кусочек какашки работает
Без комментариев.
По существу Ваш код хороший пример использования ООП там, где оно не нужно. ООП имеет 4 основных и три дополнительных отличительных признака согласно Гради Бучу. Если ничего из это нет необходимости использовать, то код лучше оставить структурным.
Скрипт делает свое дело по вполне четкой схеме. Рефакторить там нечего. Протокол в нормальных системах не меняется. Там может появиться только в самом конце код связанный с исполнением заказа. Этот код может быть уже на чем угодно в зависимости от сайта и товаров, которые он продает.
Ваш же код это откровенная бурда из ветвлений
Бурда — это неправильная логика или логика, которую можно оптимизировать. Покажите, что там можно упростить.
создания буферов вывода, отправок сообщений Игорю и записи в БД.
И что Вы предлагаете вместо всего перечисленного? Мне нужно именно email в случае ошибки и именно запись в БД.
если для вас этот пахучий кусочек какашки работает
Без комментариев.
Даже если оставить этот код процедурным, то его можно привести к куда более читаемому виду, просто разбив на функции, на первой итерации что-то типа нижеследующего получится:
А на второй, видя, что во все функции передаётся _POST возникает желание создать объект на базе $_POST. :)
Простота кода определяется не количеством циклов процессора, требуемых на его исполнение машиной, и даже не количеством строк, а легкостью чтения человеком. Представьте, достался вам этот код в наследство и вы ничего не знаете об API ЯД и видите условие
P.S.
просто
<?php
$mail_address = 'igor@itsoft.ru';
// send_notification_mail($mail_address, $_POST);
if (is_protected_payment($_POST)) {
send_protected_error_mail($mail_address, $_POST);
exit;
}
if (!is_correct_signature($_POST)) {
send_signature_error_mail($mail_address, $_POST);
exit;
}
//...
save_payment($_POST);
А на второй, видя, что во все функции передаётся _POST возникает желание создать объект на базе $_POST. :)
Простота кода определяется не количеством циклов процессора, требуемых на его исполнение машиной, и даже не количеством строк, а легкостью чтения человеком. Представьте, достался вам этот код в наследство и вы ничего не знаете об API ЯД и видите условие
if($_POST['codepro']!='false'). Вам будет понятно, что здесь проверяется? А если это же условие будет как if (is_protected_payment($_POST))? А ваши return mail(); — зачем возвращать код возврата mail, если вы его нигде не используете? Ещё ваш код очень неоднороден по стилю. Например, в одних местах вы работаете напрямую с элементами $_POST, а в других предварительно разбиваете его на переменные. Невольно начинаешь разбираться, зачем было выносить в переменные. Читаешь код несколько раз, и только после этого понимаешь, что вот такая блажь именно на этом моменте на вас нашла.P.S.
ob_start();
print_r($_POST);
$message = ob_get_contents();
ob_end_clean();
просто
$message = print_r($_POST, true);
Да вы просто на читаемость кода внимание обратите. На аккуратность, оптимальность, лаконичность. На то, как быстро другой программист сможет разобраться с этим кодом. На то, насколько вероятны будущие ошибки из-за невнимательности. В комментарии выше, все-таки логика более прозрачна чем в ваших примерах. И дело не в ООП вообще. И даже не в пхп
посмотрите на запрос. чтобы понять какие именно поля таблицы заполняются, программисту нужно залезти в базу, найти таблицу, сравнить… и упаси случай, кто-то добавит новое поле, баг вы не заметите сразу. более того скрипт возможно даже не сломается — просто будет писать не туда и не то. По этому и придумали явно перечислять поля, применять плейсхолдеры и прочее.
Да даже если бы вы написали
— элементраная этика
$r=mysql_query("INSERT INTO it_payment_ym VALUES('$operation_id', '$sender', '$amount', '$datetime', '$invoice_id')");
посмотрите на запрос. чтобы понять какие именно поля таблицы заполняются, программисту нужно залезти в базу, найти таблицу, сравнить… и упаси случай, кто-то добавит новое поле, баг вы не заметите сразу. более того скрипт возможно даже не сломается — просто будет писать не туда и не то. По этому и придумали явно перечислять поля, применять плейсхолдеры и прочее.
Да даже если бы вы написали
$r=mysql_query("INSERT INTO `it_payment_ym` SET
`operation_id`='$operation_id',
`sender'`='$sender',
`amount`='$amount',
`datetime`='$datetime',
`invoice_id`='$invoice_id'
)");
— элементраная этика
$sum = 1.005*$i->itsum;Магические константы — зло. Что такое 1.005? Почему именно столько? А вот если бы это была обычная именованная константа, то вопросов бы не возникло.
ps: если комиссия в 0.5% берётся с суммы $sum, то вы всё равно получите меньше, чем ожидаете.
Про константу согласен.
Про ps неверно. Получится то что надо. Скрипт оттестирован. Комиссия берется от суммы счета.
Про ps неверно. Получится то что надо. Скрипт оттестирован. Комиссия берется от суммы счета.
Смею полагать что предыдущий комментатор имел в виду, что 0.5% от 100,5% — это не 100%. Да, на маленьких суммах все будет хорошо за счет округления, но если говорить о крупных платежах — будете терять.
Еще раз повторяю, что скрипт оттестирован. Все работает как надо! Если счет на 10 000р. то и на ЯД.кошелек упадет 10 000р.
А если попробовать счет на 100 000 руб.?
Давайте на 100 000 и поспорим, а?
Вот там трое что минусы поставили, им тоже предложение поспорить.
Вот там трое что минусы поставили, им тоже предложение поспорить.
Легко %) 100 500 получается с комиссией? А теперь посчитайте калькулятором сколько вы от яндекса получите.
Предлагаю Вам завести авторизованный Яндекс.Кошелек, положить на него 100 500р.
Я Вам выставлю счет на 100 000р.
Вы оплатите его в нашей системе.
Если правы Вы, то я верну Вам ваши 100 000рублей + мои 100 000р., иначе, я оставляю себе Ваши 100 000р.
Я Вам выставлю счет на 100 000р.
Вы оплатите его в нашей системе.
Если правы Вы, то я верну Вам ваши 100 000рублей + мои 100 000р., иначе, я оставляю себе Ваши 100 000р.
хехе.
кажется, вы задолжали.
100000 * 0.05 = 5000 — 5% от 100к
100000 + 5000 = 105000 — это вы получите от юзера.
105000 * 0.05 = 5250 — 5% от того что видит яндекс
105000 — 5250 = -250р — ваш убыток
полагаю, m_z уже выдал вам платежные реквизиты?
кажется, вы задолжали.
100000 * 0.05 = 5000 — 5% от 100к
100000 + 5000 = 105000 — это вы получите от юзера.
105000 * 0.05 = 5250 — 5% от того что видит яндекс
105000 — 5250 = -250р — ваш убыток
полагаю, m_z уже выдал вам платежные реквизиты?
Будете счет оплачивать на 99000р.?
Счет на 100 000 нельзя выставить в силу лимита системы.
Счет на 100 000 нельзя выставить в силу лимита системы.
для того чтобы вы получили 99000, скрипт мне выставить должен
99000 * 1.05 = 103950, так?
предположим, что яндекс примет эту сумму.
и вычтет свои 5% от нее.
103950 * 0.05 = 5197,5р
тоесть вам прийдет 103950 — 5197,5 = 98752,5 и вы потеряете на этом 247,5 рублей.
что не так?
99000 * 1.05 = 103950, так?
предположим, что яндекс примет эту сумму.
и вычтет свои 5% от нее.
103950 * 0.05 = 5197,5р
тоесть вам прийдет 103950 — 5197,5 = 98752,5 и вы потеряете на этом 247,5 рублей.
что не так?
Плять этот говнокод первый в запросе google «yandex деньги php» =)
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
Автоматический прием Яндекс.Денег на сайте на php