Первый раз первый класс!
11.11.2010 мои проекты технологии
Ну наконец-то. Начал писать ещё 11 октября и вот целый месяц прошёл! Я в шоке, снова обожгло осознанием своей ужасно тормознутости. МЕСЯЦ! Это пиздец, извините.
При этом действительно работал над классом суммарно максимум сутки, наверно, куда пропало остальное время — непонятно. Ну ладно, подобных соплей и так в блоге достаточно.
Класс для работы с API AvisoSMS
Я ещё и git и github начал осваивать, в следующей заметке напишу вводную. В общем, код класса здесь — github.com/BrokenBrake/classAvisoPrefixes/. Можете просто скачать, можете отпочковывать свои ветки, слать какие-нибудь баг-репорты и всё такое. Open Source.
Скорей всего это не последняя версия, потому что я очень жду и надеюсь на критику программистов. Заметили какие-то косяки? Не молчите, пожалуйста. Это мой первый блин в ООП. Я вот уже вижу одну несуразицу: глупо выдавать ошибку при повреждённом файле кэша, его нужно просто тихонько переписать. Я так думаю.
Методы класса
Особенность реализации класса: все открытые методы выдают значение (число != 0, массив данных, строка) или информационное сообщение при нормальных условиях. Если хоть какая-то ошибка, метод возвращает False,
а значение ошибки получаем через метод errorMessage(). Кроме того есть метод errorID, возвращающий числовой код ошибки (удобно использовать для условий).
- useCache()
Использовать кэш. Можно передать массив аргументов, в котором будет желаемый относительный путь к файлу кэша (file), и/или время его жизни в секундах (live). По умолчанию значение file = var/classAvisoPrefixes.cache, а live = 86400 (кэш будет обновляться раз в сутки). Если вы не используете проверку цены SMS на номер, смысла в кэше нет. Естественно, файл кэша должен быть доступен для записи. - errorMessage()
Получить значение ошибки (False, если ошибок не было). - errorID()
Получить цифровой код ошибки (аналогично errorMessage). - allNumbers([$coverage])
Получить массив всех возможных номеров. О ключах и значениях не буду здесь, можете прочитать в комментариях класса, прямо в начале исходников. В метод можно передать необязательный параметр-фильтр $coverage, со значениями: ru, ua, ru-ua (подробности там же). - numbersAroundCost($sum [, $coverage])
Получить короткие номера телефонов стоимостью для абонента в районе $sum рублей (с НДС). Выдаёт массив из двух ближайших номеров, формат массива аналогичен allNumbers + добавляется параметр diff – отличие цены от заданной суммы. Вторым параметром также может передаваться охват (ru, ua, ru-ua). - profitFromNumber($number)
Средняя сумма выплаты вебмастеру (в рублях). - costForNumber($number)
Формат аналогично profitFromNumber, но выдаёт максимальную для абонента цену. - costFromTo($number)
Получить строку вида «от N руб. Z коп. до X руб. Y коп.» для номера. Естественно, это значения конечных цен для абонента (с НДС). Удобно использовать в подсказках абоненту, не обрабатывая результат
выдачи costForNumber(). - checkSignal($signal, $check [, $reply])
Проверить сигнал от AvisoSMS. Первым аргументом удобно делать массив $_REQUEST, во втором массиве вы можете передать любые параметры, которые хотите сравнить. Для справки читайте описание API. Метод возвращает $reply (OK по умолчанию) или False.
Что дальше?
Ругайте, пожалуйста! Хорошенько. Очень нуждаюсь в толковой критике, повторяю — это мой первый блин, поэтому наверняка я налепил много новичковых глупостей. Тем не менее, класс полностью работоспособен, я протестировал его. Класс выполняет свою работу.
В ближайшее время планирую обновление Йерки с его использованием и ещё выход совершенно нового продукта (в ограниченном тираже). Следите за блогом. В следующей заметке напишу про github.
Комментарии
Комментирование этой статьи закрыто.
« Начало работы с Git и GitHub Поддержим путешественников? »
Мельком просмотрел класс. В именах методов, насколько я знаю, принято указывать явнее действие. То бишь глагол + существительное. Типичные действия — get, put.
У тебя пачка методов зовётся просто существительными: allNumbers, errorID. При этом другие методы зовутся checkSignal, readCache и getPipe. А если надо будет setAllNumbers — то как потом разбираться, что получить — это allNumbers а не getAllNumbers?
ЗЫ: как вообще с гитом работать было?
У меня английский очень хромает. С именами, действительно, не всё гладко, согласен.
С гитом сперва нихрена не понятно, а потом вроде ничего… думаю, привыкну, начать сложно.
Если уж совсем по уму делать, надо в случае ошибки генерить эксепшен, а не возвращать фолс. А то как булевское значение вернуть? :)
False в случае ошибки.
0. Самое, на мой взгляд, ужасное – нет тестов. Это ведь не только самоконтроль и гарантия того, что код делает всё положенное, но и неплохая документация по API с use-кейсами.
1. Работа с ошибками: может, не стоит состояние ошибки хранить в экземпляре класса? В случае чего непредвиденного, класс мог бы бросаться исключениями с нужным сообщением и кодом ошибки.
2. Методы класса длинноваты. Чисто субъективно: если метод занимает больше 10-20 строк, его надо дробить на более мелкие. Рекомендуется методы делать атомарными, то есть один метод – одна маленькая неделимая функциональность. Эти лямбды, которые нифига не лямбды, тоже можно было бы вынести, пожалуй. Кстати, в эру php5.3 можно было бы не ущербный create_function использовать, а нормальные анонимные функции :)
3. Класс в целом получился великоват. Классы, как и методы, обычно рекомендуют максимально минимизировать (о как). То есть не надо впихивать весь функционал в тело одного класса.
Впрочем, вероятно, это уже тема следующей серии.
4. Не стоит хардкодить строки и числа в коде. Это касается и мифической пятёрки на 481-й строке, и к урлу пайпа. Ну а про коды ошибок уже в коде отмечено.
Про стандарты оформления и именования ничего говорить не буду, это всего лишь рекомендации. Главное – придерживаться одного стиля.
Сходу больше ничего не придумывается :)
Блокировки ошибок с помошью @ – зло, где можно лучше дополнительную проверку сделать.
Для критических ошибок лучше Exceptions сделать, можно даже свой класс исключений сделать что бы потом отлавливать удобнее.
По поводу констант ошибок – сделай константами класса в чем проблема?
const FETCH_DATA_ERROR = 1;
и потом везде AvisoPrefixes::FETCH_DATA_ERROR
AvisoPrefixes, юнит тесты при работе со сторонними сервисами спорная штука, все равно нужен ручной контроль.
/*********************
- Получить номер(a) для цены
*/private $limit = 2; // сколько номеров в выдаче?
public function numbersAroundCost($sum, $coverage = ‘’)
жесть… если уж объявляешь так жестко – вынеси в начало класса.
>Блокировки ошибок с помошью @ – зло, где можно лучше дополнительную проверку сделать.
Полностью согласен. try..catch в помощь, если не уверен.
2marazmiki – по 3-му пункту не согласен с тобой. Класс – это одна сущность и все действия должны быть заложенны в данном классе. Другое дело, что большую сущность обычно можно разбить на несколько мелких.
Зачем юзаешь трубы? Без них никак?
Хорошо бы еще все в phpDoc завернуть. Ну и юнит-тесты да, тоже неплохо бы.
aktuba, так тут явно несколько сущностей! И классов должно быть несколько.
Как уже говорили выше – есть смешивние ОПП подхода и процедурного.
Получение ошибки с помощью вызова дополнительной функции собственно как раз процедурный подход.Если уж использовать ООП – то либо эксепшины (они есть в php?), либо возвращать объект нужного типа.
Т.е. если фукция возвращает эземпляр класса ЧтоТоТамОшибка – то это ошибка. И сама ошибка храниться в нем.
Это позволит легко и просто передавать ошибки на уровень выше или вообще в другой модуль.
Достаточно логично обработку ошибок вынести в отдельный модуль, который ничего не обязан (и не должен знать) о основном приложение. Все что он должен принимать объекты определенного типа и обрабатывать их. При текущем подходе этот модуль должен будет иметь доступ к основному классу.
Evgeny Sergeev, в данном случае – да. Как минимум – кеш необходимо вынести. А вообще, правильнее сделать абстрактный класс (интерфейс), который выполняет общую работу (инициализация, подключение кеша и т.д.) и наследоваться от него. Но это уже выходит за рамки данного поста – тут Тормоз опубликовал первый свой класс, а не либу =).
Всем спасибо! Сейчас внимательно прочту и отвечу. У меня отключали электричество (как в деревне, блин, два раза!), поэтому я решил поспать :)
Marazmiki, к тестам я пока не пришёл, но понимаю, что нужно. Скорей всего в следующей затее уже буду использовать методику TDD. Со всеми остальными рекомендации почти полностью согласен, есть ещё место для шага вперёд.
Джек, я сейчас уже не помню что именно не получилось, но почему-то с константами класса не вышло. В следующий раз попробую. А ещё про собачку — ну зачем городить проверку и вывод ошибок для простейшего счётчика ++$ID? Это же совсем элементарная штука. И вторая собачка в file_put_contents тоже оправдана, потому что ошибка с доступом выявляется до вызова этой функции. Я так думаю.
Aktuba, как раз всё помещать в начало — дурной тон (хотя и такое у меня есть). Я согласен с тезисом о том, что переменная должна объявляться максимально близко к месту использования.
Я так не умею ещё пока :) Но обязательно научусь, скорей всего это на самом деле правильный подход. Читал об этом в книгах.
Какой-то странный вопрос. Без них КАК, но с ними элементарно удобнее. Назови хоть одну причину, по которой мне не стоит пользоваться любимыми Yahoo Pipes.
Что в целом? Как для первого блина, совсем всё плохо или бывает хуже? :)
>> Если хоть какая-то ошибка, метод возвращает False,
а значение ошибки получаем через метод errorMessage().
Дальше не читал
>Что в целом? Как для первого блина, совсем всё плохо или бывает хуже? :)
Тормоз, то что бывает хуже – факт.
У меня предложение – давай вместе отрефакторим этот класс?
Эх, Миша, Миша. Вроде адекватный был такой, а ведёшь себя как тролль :) Я же просил покритиковать, а не просто кидать помидорами. Чего тебе не понравилось?
Евгений, давай! А как вместе? Я не очень представляю себе синхронизацию этого процесса. Я планировал начать писать класс-кэшер, а потом уже переделать этот. В письме написал.
ЮAktuba, как раз всё помещать в начало — дурной тон (хотя и такое у меня есть). Я согласен с тезисом о том, что переменная должна объявляться максимально близко к месту использования.
Ну и зря… У меня привычка еще с Delphi осталась и очень помогает:
1. сначала описываем константы
2. описываем переменные (public, protected, private – именно в таком порядке)
3. описываем методы (private, protected, public).
В итоге, я точно знаю где искать нужные метод или параметр. Объявляя же какой-то параметр где-то между методами, через пару месяцев запаришься искать где он объявляется, какого он типа и т.д.
Тесты и TDD не одно и то же. TDD в общем случае эффективнее по трудовым и временным затратам, тесты могут писаться и постфактум.
Собачки в коде там, где без них можно обойтись – действительно зло. В том числе по соображениям ресурсоёмкости. Фактически одна собачка суть локальное переключение error_reporting‘а на E_NONE и обратно на предыдущее значение. Это типа затратно.
Aktuba, ну ты значит не владеешь пока приёмами эффективного редактирования исходных текстов, раз у тебя такая вредная привычка :) Vim, например, позволяет мгновенно переключаться на нужные строки.
Про красоту кода мне нравится статья от Алика Кирилловича. Кстати, ты там вроде был в комментах, не помню, давно читал.
Marazmiki, обязательно приду к тестам и обещаю отказаться от собачек :)
Тормоз, если у тебя переменная используется в одном методе – то это нормально ее объявить рядом с этим методом. Но все же, если она используется в нескольких методах ( иначе зачем ее объявлять вне метода???) – то понятие близко к месту использовать становиться не очевидным.
Плюс если переменные можно разнести логически на несколько подтипов – вот тут константы, вот тут строковые переменные, вот тут экземпляры такого класса – то все же лучше их группировать и выносить в начало класса.
А Мишу прости, ибо получении ошибки через errorMessage (getErrorMessage тогда уж) и ООП – две сущности которые в одном предложении выглядят дико. И навивают мысли что ООП для котого это просто использование классов, при старом процедурном подходе :)
Я не знаю зачем нужны трубы. Мне лениво там регистрироваться. У меня сложилось впечатление, что они нужны тем, кто не умеет программиировать. Какую функцию они выполняют у тебя?
о, прикольно :) Правда APIv2 выйдет скоро, но для разминки неплохо)
А между тем, @ на некоторые файловые операции есть прямо в примерах в официальном мануале (и я вроде даже её видал в бекап-модуле Zend Framework).
Тормоз, да, поведай нам — что там в пайпах вообще происходит? А то проект опенсорсный, а часть функционала спрятана.
Young, блин! Ну так ты скажи как надо делать в этом случае? :) Какое твоё мнение по поводу вывода ошибок в ООП? Try/catch?
Polonskiy, у тебя сложилось неверное впечатление. Но в данном случае, в принципе, можно было бы обойтись и без Pipes, однако мне так просто удобней. Трубка конвертирует json в сериализованные данные PHP и отфильтровывает российские и украинские номера (Беларусь остаётся за кадром).
ASH, так, а тут подробнее, пожалуйста! Что за APIv2? Надеюсь, старое API при этом не накроется?
Samlowry, да ничего не спрятано — смотрите. URL же есть в исходниках.
Дык, я же вроде сказал. Либо библиотеку делать с использование экцепшинов. Т.е. при ошибках кидать нужные экцепшины (т.е. по сути объектны некоторые).
Либо возвращать некоторые объекты руками – т.е. по сути более простой аналогичный механизм.
В яве это бы выглядело бы так.
1. В варианте с exception – бы бы сделал несколько класссов наследников (CacheInitExeption и т.п.) от стандартного класса exception и бросла бы их (throw) в случае ошибок, а там бы пользователь библиотеки смотрел бы ловить их или нет и на каком уровне.
2. Либо, что возможно к php ближе, хотя мне не нравиться – возвращал бы объекты бы определенных классов.
Т.е. опять же бы в яве было бы так
Object response = numbersAroundCost
if (response instanceof AvisoErrorRespone) sendErrorToVisualizator(response)
numbersCost->getNumberCostByShortName(«premium»); }else
{ AvisoNumbersCost numbersCost = (AvisoNumbersCost) response;
P.S. Кстати по поводу труб и программирования. Я лично считаю что это нормально в первой версии использовать различные вспомогательные вещи с которыми знаком, даже если возможно они работают не самым оптимальным способом. Степ бай степ – это самое главное.
ОК, буду изучать это всё дело. Спасибо. А чем именно такой подход лучше, чем мой errorID()?
Тем что у тебя сообщение о ошибке отчуждаемый элемент и его можно дальше передать по цепочке.
Смотри сейчас мы создали экземпляр твоего класса, сделали запрос – получили ошибку. Далее теперь нам нужно это ошибку где-то залогировать или где-то отобразить. Нам нужно передать экземпляр твоего класса в логер, и там его спросят об ошибке и выведут ее. А если у нас асинхронное использование? И во время между вызовом одиного метода и визуализацией его ошибки вызовут другой метод – все информация о ошибке либо потерянна, либо неверна что еще хуже.
Далее опять же мы можем сделать запрос у твоего класса, получить ошибку и сразу его удалить – а тут придеться хранить до тех пор пока не запросим ошибку.
В конкретном случае и класс не большой, и ошибка в виде константы (а может быть более сложной – содержать в себе например полностью дамп состояния объекта) – можно и класс передать и вообще константу скопировать. Но у нас же есть промежуточная задача в ООП потренироваться – то почему бы правильно не сделать.
Т.е. в данном конкретном случае (особенно если не думать о расширении) подход не лучше, а просто более правильный.
Далее, смотри. Допустим – у нас нужно обрабатывать различные ошибки на различном уровне.
Т.е. смотри есть глобальная система – в ней модуль – вней подмодуль – и уж в подмодуле вызывается твой класс
Допустим мы хотим часть ошибок обрабатывать в подмодуле, часть в моделе- например если что отключить подмодуль, и часть в глобальной системе – перезагрузить все нафиг например.
С экцепшинами это все достататочно элегантно делается.
Т.е. ты часть экцепшинов перехвытываешь, часть пропускаешь на более высокий уровнь и так далее.
Ручками это сделать – не так просто, везде городить после вызова проверку if (getError())
{
} else
{
}
Тяжко это.
Ну и еще можно много чего вспомнить…но 2 часа ночи тут у меня. Это все конечно может где-то оверхед для конретно это задачи – но помоему вполне ложиться сюда.
Проясняется, теперь почти всё понял. Спасибо! Стану умнее, следующая версия будет лучше :)
Тормоз, нет, не накроется, будет работать параллельно, но во второй больше вкусностей. Как будет разрешено показывать – пришлю линк.