Комментарии 14
Не ради холивара:
Зачем нужен интерфейс, если у него ровно одна авторизация.
Зачем нужен интерфейс, если у него ровно одна авторизация.
Ровно одна «реализация» может быть?
Это было сделано в большей степи для того, чтобы описание фукнциональности было в одном месте, чтобы легче было понимать его возможности.
Опять же, если кого-то эта реализация не устраивает, они могут написать свою или изменить существующую. Ну и мне кажется, это хороший тон отделять описание поведения от реализации. Здесь это может и не нужно, но в будущем такое восприятие может уберечь кого-нибудь от потенциальных граблей.
Это было сделано в большей степи для того, чтобы описание фукнциональности было в одном месте, чтобы легче было понимать его возможности.
Опять же, если кого-то эта реализация не устраивает, они могут написать свою или изменить существующую. Ну и мне кажется, это хороший тон отделять описание поведения от реализации. Здесь это может и не нужно, но в будущем такое восприятие может уберечь кого-нибудь от потенциальных граблей.
а зачем вам было что-то править в денвере? он хорош как раз тем что работает из коробки
Мне хотелось достучаться до него по локалке. А там из коробки это как-то проблематично. А вообще у всех WAMP'ов (LAMP'ов) все работает из коробки. Тут дело вкуса.
там это делается одним движением ноги, и даже описано в факе: www.denwer.ru/faq/shared.html
директорию переименовал, перезапустил и погнал…
директорию переименовал, перезапустил и погнал…
Автор, простите, но ваша библиотека (и статья) — это ужас.
Статья
В статье вы говорите, что ни разу до этого не работали с PHP — и это заметно. Хорошо хоть, вы этого не скрываете. Я не понимаю, как это — вы не смогли разобраться с тем, как в Денвере прописать возможность доступа извне, и, видимо, вообще не читали описание Денвера, и как с ним работать :). Как работнику Яндекса, вам должно быть стыдно. Потому что в Денвере можно реально поменять всё, достаточно просто прочитать описание, как это устроено и как работает (и почему именно так). Свои вхосты в denwer тоже добавить очень легко — достаточно в httpd.conf их прописать
О проекте на github
В описании сказано следующее:
Не сказано ни-че-го про Mac OS X, а также описываются действия, которые человеку, который хотя бы один раз работал с PHP, объяснять не надо :). То есть, по сути, шаги 2 и 3 — неправильны. Второй шаг неверен потому, что, во-первых, у человека уже может стоять PHP, а во-вторых потому, что можно, скажем, на удаленный хостинг скопировать файлы и не устанавливать себе ничего. Третий шаг неправилен потому, что «каталог www» может называться как угодно, и, как правило, действительно называется не www.
Следующий код просто ужасен:
Чем вас не устроил echo :)? Про <br> я уж промолчу.
Реализация класса
Про интерфейс вам уже сказали, это не самое страшное :).
Для начала, отметим, что вы не используете неймспейсы (в принципе, это логично, если вам нужна совместимость с версиями PHP ниже 5.3)
1)
Начнем с того, что в include_path может быть первой директорией отнюдь не ".", а что-нибудь другое, и файл может не заинклюдиться.
Потом, класс Crypt_AES может уже присутствовать в системе, и файл не нужно инклюдить, если этот класс уже есть (иначе будет Fatal error).
Ну, и include() в данном случае лишь сгенерирует warning, если файл не получилось подключить, а вашему классу этот файл _необходим_ для работы, поэтому здесь нужно писать require().
Итого, правильный код для включения класса AES будет следующим:
2)
Вы никогда не задумывались над тем, что у кого-нибудь в проекте могут быть уже объявлены эти классы :)? Поскольку неймспейсы вы не используете, то это, опять же, будет Fatal error в PHP.
3)
Я честно скажу, я сильно не уверен, что этот код действительно нужен, потому что print_r($this) выдал бы почти тот же самый результат. Вы, наверное, перепутали PHP с C#/Java
4)
Библиотека общего назначения не должна отправлять header'ы ни в коем случае, ибо это является грубейшим нарушением MVC и это поведение будет несовместимо с распространнеными PHP-фреймворками, да и вообще любым более-менее сложным кодом на PHP.
Даже если вы очень хотите оставить header(), нужно поставить после него die(), ибо просто посылка header('Location: '); не прекращает исполнения скрипта в PHP после перенаправления. В любом случае, как максимум, библиотека должна возвращать URL, куда перекинуть пользователя, не больше.
В общем, возможно, вас в Яндексе уже отругали за ваш код, но я просто хотел бы вам указать на явные недостатки, которые, ИМХО, совершенно недостойны такой компании, как Яндекс
Статья
В статье вы говорите, что ни разу до этого не работали с PHP — и это заметно. Хорошо хоть, вы этого не скрываете. Я не понимаю, как это — вы не смогли разобраться с тем, как в Денвере прописать возможность доступа извне, и, видимо, вообще не читали описание Денвера, и как с ним работать :). Как работнику Яндекса, вам должно быть стыдно. Потому что в Денвере можно реально поменять всё, достаточно просто прочитать описание, как это устроено и как работает (и почему именно так). Свои вхосты в denwer тоже добавить очень легко — достаточно в httpd.conf их прописать
О проекте на github
В описании сказано следующее:
Для успешного запуска примеров из комплекта следует проделать следующее:
* зарегистрировать приложение, т.е. получить идентификатор клиента (https://sp-money.yandex.ru/myservices/new.xml) и прописать его в константы примеров (consts.php);
* установить какой-нибудь WAMP (Windows-Apache-MySQL-PHP) или LAMP сервер (http://en.wikipedia.org/wiki/Comparison_of_WAMPs);
* скопировать файлы примеров и библиотеку в каталог www.
Не сказано ни-че-го про Mac OS X, а также описываются действия, которые человеку, который хотя бы один раз работал с PHP, объяснять не надо :). То есть, по сути, шаги 2 и 3 — неправильны. Второй шаг неверен потому, что, во-первых, у человека уже может стоять PHP, а во-вторых потому, что можно, скажем, на удаленный хостинг скопировать файлы и не устанавливать себе ничего. Третий шаг неправилен потому, что «каталог www» может называться как угодно, и, как правило, действительно называется не www.
Следующий код просто ужасен:
print_r('Номер счета: ' . $accountInfoResponse->getAccount() . '<br>'); print_r('Баланс: ' . $accountInfoResponse->getBalance() . '<br>'); print_r('Код валюты: ' . $accountInfoResponse->getCurrency() . '<br>');
Чем вас не устроил echo :)? Про <br> я уж промолчу.
Реализация класса
Про интерфейс вам уже сказали, это не самое страшное :).
Для начала, отметим, что вы не используете неймспейсы (в принципе, это логично, если вам нужна совместимость с версиями PHP ниже 5.3)
1)
<?php
/*
* Модуль для простой и удобной работы с API Яндекс.Деньги.
* Использует библиотеки: cUrl, Mcrypt.
*/
include('AES.php'); // модуль шифрования
Начнем с того, что в include_path может быть первой директорией отнюдь не ".", а что-нибудь другое, и файл может не заинклюдиться.
Потом, класс Crypt_AES может уже присутствовать в системе, и файл не нужно инклюдить, если этот класс уже есть (иначе будет Fatal error).
Ну, и include() в данном случае лишь сгенерирует warning, если файл не получилось подключить, а вашему классу этот файл _необходим_ для работы, поэтому здесь нужно писать require().
Итого, правильный код для включения класса AES будет следующим:
if(!class_exists('Crypt_AES')) {
requre(dirname(__FILE__).'/AES.php');
}
2)
/**
* Класс-перечисление прав приложения на использование эккаунта Яндекс.Денег
* пользователя.
*/
class Scope {
...
/**
* Класс для получения информации о конкретной операции из списка операций
* метода operationHistory. Используется в объекте OperationHistoryResponse
* @author dvmelnikov
*/
class Operation {
...
Вы никогда не задумывались над тем, что у кого-нибудь в проекте могут быть уже объявлены эти классы :)? Поскольку неймспейсы вы не используете, то это, опять же, будет Fatal error в PHP.
3)
/**
* @return string возвращает содержимое объекта в качестве строки
*/
public function __toString() {
$parent = parent::__toString();
return "OperationDetailResponse{error: $this->error, details: $this->details,
sender: $this->sender, recipient: $this->recipient, message:
$this->message, codepro: $this->codepro}" . $parent;
}
Я честно скажу, я сильно не уверен, что этот код действительно нужен, потому что print_r($this) выдал бы почти тот же самый результат. Вы, наверное, перепутали PHP с C#/Java
4)
class YandexMoney implements IYandexMoney {
...
public static function authorize($clientId, $scope = NULL, $redirectUri = NULL) {
if (!isset($clientId) || $clientId == '') {
throw new YandexMoneyException(YandexMoneyException::ERR_MESS_CLIENT_ID, 1001);
}
if (!isset($scope) || $scope == '') {
$scope = Scope::ACCOUNT_INFO . Scope::OPERATION_HISTORY;
}
$scope = trim($scope);
header('Location: ' . self::URI_YM_AUTH . "?client_id=$clientId" .
"&response_type=code&scope=$scope&redirect_uri=$redirectUri");
}
Библиотека общего назначения не должна отправлять header'ы ни в коем случае, ибо это является грубейшим нарушением MVC и это поведение будет несовместимо с распространнеными PHP-фреймворками, да и вообще любым более-менее сложным кодом на PHP.
Даже если вы очень хотите оставить header(), нужно поставить после него die(), ибо просто посылка header('Location: '); не прекращает исполнения скрипта в PHP после перенаправления. В любом случае, как максимум, библиотека должна возвращать URL, куда перекинуть пользователя, не больше.
В общем, возможно, вас в Яндексе уже отругали за ваш код, но я просто хотел бы вам указать на явные недостатки, которые, ИМХО, совершенно недостойны такой компании, как Яндекс
Спасибо вам за развернутый ответ. Рекомендации учту и поправлю код.
YandexMoney::authorize(Consts::CLIENT_ID, 'account-info operation-history', Consts::REDIRECT_URL);По-моему ужасно нечитабельный интерфейс — трудно предположить как вызвать функцию не прочитав инструкцию, если она конечно имеется. Имхо гораздно удобнее использовать ключ-значение:
YandexMoney::authorize(array('client_id' => Consts::CLIENT_ID, ...));Зарегистрируйтесь на Хабре, чтобы оставить комментарий
PHP-библиотека для работы с API Яндекс.Денег