getjump / VkApiPHP

[Abandoned] Library for work with API Vk.com
MIT License
201 stars 53 forks source link

Рекомендации #2

Closed yakud closed 10 years ago

yakud commented 10 years ago

Было бы здорово не только PSR-0 реализовать.

Подобную проверку:

public function each($callback) {
  if (!is_callable($callback))
    return;
  ...
}

Можно заменить на аргумент со строгим типом:

public function each(Closure $Callback) {
   ...
}

Очень бы хотелось строго описанных аргументов функций. Например:

function fn(array $data) {...}
function fn(User $data) {...}
// etc...

class Error {
....
}

=>

class Error extends Exception  {
....
}

По PSR'у будет правильнее:

if (condition) event
// =>
if (condition) {
    event
}

public function __construct($methodName, $args = false, Core $vk) { ... }

В таких случаях лучше делать так:

public function __construct($methodName, array $args = array(), Core $vk) { ... }

Можно невзначай посмотреть на PHPDoc и отдать true. А внутри проверка:

if ($args) {...}

Ну и конечно, не хватает тестов.

yakud commented 10 years ago

Хотелось бы увидеть README.md не в капсе

Sett commented 10 years ago

Было бы здорово заменить входящие параметры адекватными переменными, а не одно-символьными.

getjump commented 10 years ago

@Sett Что вы подразумеваете под входящими параметрами?

Sett commented 10 years ago

Математические переменные не самый удобочитаемый вариант для именования объектов. Речь как и на хабре об анонимных функциях. Но я уже понял, что в этом плане можно форкнуться и уродовать, ну т.е. приводить к своей красоте самостоятельно.

getjump commented 10 years ago

@Sett Я так понял, что вы говорите о ПРИМЕРЕ использования. Ни что не заставляет вас использовать именно его, вы можете переименовать эти "гадкие" переменные в что либо вроде $user, $data, $friend и т.д. each, имеет туже логику, что и foreach, первый аргумент анонимной функции - id объекта, а второй это элемент из множества, соответственно $i, $k, $j - неплохой выбор для номера элемента. Скажем, если вам покажется, что ваш код оптимальней, наглядней и т.д., то будет любезно с вашей стороны, сделать Pull Request, возможно ваше мнение будет разделено.

andycaramba commented 10 years ago

У вас замечательная библитека, особенно по сравнению с другими сущетвующими на данный момент PHP либами для доступа к VK API. И за это вам огромное спасибо. Только вот есть один момент, а именно требование PHP минимум версии 5.5. Я так понял это сделано ради одного единственного yield в RequestTransaction.php? Если так, то эта часть легко переписывается на тот же итератор, что позволяет юзать более раннюю версию PHP, например ту же 5.4, которая пока весьма актуальна. Да, я читал, что yield выигрывает в производительности, но в данном конкретном случае она ведь будет практически не заметна.

getjump commented 10 years ago

@andycaramba теперь минимальная версия 5.4

andycaramba commented 10 years ago

@getjump замечательная новость. большое спасибо.