Помогите исправить собственное быдлокодерство

[craz]

Откуда может появиться 70 проверок на странице я вот не могу понять? Один же раз проверяем, потом просто пользуемся переменной, которая хранит значение да/нет...

 

[Фанат]

Сделаю аналогично, но пока без pdo.

Это не PDO. Это SafeMySQL

пользуюсь mysql_query (книги мои по php датированы до 2007-го года и там других способов подключения к БД не было). Понимаю что это каменный век, не безопасно и т.п.

mysql_query может быть не менее безопасной и более удобной, чем PDO, если делать с умом.
PDO, в свою очередь - не гарантия безопасности.

 

[WMix]

Откуда может появиться 70 проверок на странице я вот не могу понять? Один же раз проверяем, потом просто пользуемся переменной, которая хранит значение да/нет...

у тебя другой уровень, но чтоб ответить на твой вопрос, один раз в контроллере, 15 раз в разных моделях, 10 раз во вью и во всех _partials остальное понятия не имею, возможно в bootstrape а возможно в хелперах если говорить о zend

 

[craz]

у тебя другой уровень, но чтоб ответить на твой вопрос, один раз в контроллере, 15 раз в разных моделях, 10 раз во вью и во всех _partials остальное понятия не имею, возможно в bootstrape а возможно в хелперах если говорить о zend

ты так пишешь на ЗЕНДЕ??? О_О, http://framework.zend.com/manual/1.12/en/zend.registry.using.html

 

[fixxxer]

Registry - это просто замаскированный globals. Соответственно, использовать можно только для совсем центральных вещей, типа логгера или пула соединений, и set() может вызываться только в просессе бутстрапинга приложения. Registry::set в, скажем, контроллере - это то же самое, что global.

Я это считаю антипаттерном, появившимся в java для эмуляции глобальных переменных, которых там нет. В php достаточно обернуть $GLOBALS в ArrayObject чтобы получить ровно то же самое.

 

[WMix]

craz
у меня может довольно таки часто встречаться конструкция User::getInstance($id=null)->isAllowed($modul).
Zend_Registry::get('permissions') чемто отличается от isAdmin($user_id)? а этим ты только получишь масив, его еще нужно обработать чтоб понять isAdmin ли $user_id

Zend_Registry использую только для конфига

 

[craz]

craz
у меня может довольно таки часто встречаться конструкция User::getInstance($id=null)->isAllowed($modul).
Zend_Registry::get('permissions') чемто отличается от isAdmin($user_id)? а этим ты только получишь масив, его еще нужно обработать чтоб понять isAdmin ли $user_id

Zend_Registry использую только для конфига

Ну так если сложить туда именно Acl, кстати сам Acl, как устроен?

 

[WMix]

acl ничего не говорит о пользователе,.. это список роль:access, User это дополнительная оболочка на Zend_Auth которая использует Acl

 

[craz]

acl ничего не говорит о пользователе,.. это список роль:access, User это дополнительная оболочка на Zend_Auth которая использует Acl

Ну я про то что зачем ты дергаешь юзера и узнаешь у него какие-то поля, если можно дергать Acl и узнавать роль пользователя.

 

[WMix]

в зависимости от места... иногда интересен конкретный пользователь, и от него интересуют не только права, но и к примеру фамилию, иногда id иногда группы иногда права... все это я запихал в класс User
иногда меня интересует не конкретвый пользователь, а пользователь с $id = 100049 я напишу User::getInstance(100049) а дальше опять в зависимости от того что меня интересует у данного пользователя...

зы может откроем новую ветку "как мы пишем на зенде" чтоб от темы не отходить!

 

[craz]

в зависимости от места... иногда интересен конкретный пользователь, и от него интересуют не только права, но и к примеру фамилию, иногда id иногда группы иногда права... все это я запихал в класс User
иногда меня интересует не конкретвый пользователь, а пользователь с $id = 100049 я напишу User::getInstance(100049) а дальше опять в зависимости от того что меня интересует у данного пользователя...

зы может откроем новую ветку "как мы пишем на зенде" чтоб от темы не отходить!

вот вот, в теме то он проверяет принадлежность к группе! не надо новую ветку.

Я про то и говорю что не может быть там 70 вызовов - это признак не правильной архитектуры.

 

[WMix]

погоди, а как ты проверяешь в какой группе?

 

[craz]

погоди, а как ты проверяешь в какой группе?

Не важно как, важно что это делается в главном контролере, или же в хелпере каком нибудь, из бутстрапа.

 

[gray]

я честно говоря не понимаю правда твоего кода...

function isAdmin($id){ 
        if (!isset($truefalse[$id])) {
            $sql="SELECT 1 FROM `users` `t1`, `groups` `t2` WHERE `t1`.`id`='".mysql_real_escape_string($id)."' AND `t2`.`sysName`='admin' AND `t1`.`group_id`=`t2`.`id`;";
            $query=mysql_query($query) or die ("Ошибка выполнения запроса к БД: ".mysql_error());
            $result=(bool)mysql_num_rows($result);
        }
        return $result;
}

Еще раз мне объясните почему не так? Из-за мефического хайлоада?

Аааа я понял тут архитектура кривая, блин, а я то... isAdmin должен браться из сессии пользователя, а проверяться чуть ли не в бутстрапе.

Короче как всегда проблема свелась не к тому КАК ты пишешь, а ЧТО ты пишешь...

Хайлоад - когда сервер перенагружен, так?

логика такая:
мне нужно что-то вывести для сотрудника, но не менеджера и логиста -

if(isCoworker($id)&&(!isManager($id)||!isLogist($id))

Затем мне нужно вывести что-то другое с другими условиями для всех сотрудников, кроме менеджеров -

 if (isCoworker($id)&&!isManager($id))

А внутри второго if() еще дополнительное условие: не выводить что-то, помимо перечисленных логисту

if(!isLogist($id))

Зачем же мне повторно обращаться к БД за теми же самыми данными, если я их уже получил десятью строками ранее.
Естественно ни о каком перегрузе сервера не идет речь. Вся моя программа спокойно могла бы работать на домашнем сетевом хранилище. Но делаю так, потому что не вижу необходимости нагружать mysql, просто потому что могу нагрузить и он выдержит это.
И еще в будущем, возможны эксперименты с другими БД, а я хотел бы оставить возможность разделять сервера хостинга и БД при этом не теряя время на получения ответа от удаленной БД.

Да, $id берется из сессии, что позволяет мне вообще ничего не подавать на вход функций. Но, как написал выше, возможно буду обрабатывать список пользователей внутри программы, а значит проверять можно будет, последовательно подавая на вход идентификаторы, а не ид пользователя из сессии.

На самом деле не обязательно применять один и тот же подход.
То есть, если делать унифицированно, то обязательно:

function checkRights($is,"admin")

- здесь код должен быть единообразным для всех вариантов. Но зато количество функций уменьшится.

function inGroup($user_id, $group='admin')

Вполне логично и разумно. Однообразные функции не нужны. Оставил их из-за того, что в eclipse очень удобно пользоваться автоматической вставкой переменных и функций. Т.е. вводишь часть названия функции, а затем курсором просто выбираешь нужную. И тебе не нужно нигде вводить дополнительно текст группы, в написании которого могу допустить ошибку. ) я так частенько в программе делаю. внимательность конечно хорошо, но если незаметил, а у тебя все время на выходе false из-за того что не было такого идентификатора группы (теперь используется sysName).
Но когда разработка программы будет уже не такой частой и все будет более менее работать нормально, то я естественно сделаю вместо этих функций одну для сотрудников, а другую для клиентов.

Но если на каждый чих используется своя функция, то почему бы не сделать для админов и прочей мелочи заполнение массива целиком, а для юзеров - вот такой вариант.
Хотя на странице, где идёт 70 проверок, скорее всего просто неверно построен запрос к БД.

Логично и опять спасибо за идею. Сотрудники часто ходят по страницам и им нужно максимально быстрый вывод информации, а следовательно для сотрудников логично будет вообще сократить обращение к БД до одного в данной функции. У пользователей задача проще и это, наверное, не критично.

Что касается запросов к БД, то согласен с тем что неправильно. После того, как функция приняла такой вид:

static $return;
		if (!isset($return[$id])) {
			$query="SELECT 1 FROM `users` `t1`, `groups` `t2` WHERE `t1`.`id`='".mysql_real_escape_string($id)."' AND `t2`.`sysName`='admin' AND `t1`.`group_id`=`t2`.`id`;";
			$result=mysql_query($query) or die ("Ошибка выполнения запроса к БД: ".mysql_error());
			$return[$id]=(bool)mysql_num_rows($result);
		}
		return $return[$id];

кол-во запросов к бд для функции isAdmin с 31-го сократилось до 1. Логично, что по аналогии и кол-во обращений у функции isClient сократится.

 

[WMix]

Не важно как, важно что это делается в главном контролере, или же в хелпере каком нибудь, из бутстрапа.

во viewHelper, меня интересует разрешен ли пользователю тот или иной ресурс, чтоб показать/скрыть линк ты это как делаешь?

 

[craz]

Реальное у меня ощущение что тебе нужен ACL и наследование ролей, чтобы не городить if(isCoworker($id)&&(!isManager($id)||!isLogist($id))

 

[WMix]

если что if (isCoworker($id)&&!isManager($id)) обратится к базе 2 раза...

 

[craz]

во viewHelper, меня интересует разрешен ли пользователю тот или иной ресурс, чтоб показать/скрыть линк ты это как делаешь?

<?
if ($this->isAdmin){
echo "link";
}
?>

 

[gray]

если что if (isCoworker($id)&&!isManager($id)) обратится к базе 2 раза...

Да, но при следующем вызове одной из этих двух функций не обратится ни разу.

Реальное у меня ощущение что тебе нужен ACL и наследование ролей, чтобы не городить if(isCoworker($id)&&(!isManager($id)||!isLogist($id))

Нашел вот это. http://habrahabr.ru/post/51327/
Спасибо. Ознакомлюсь.

 

[WMix]

в таком случае у тебя в каждом Action записана строка $this->veiw->isAdmin
а еще в каждую $form и в каждой ControllerHelper и в каждый ActionHelper нужно тоже передавать с таким подходом... не геморойно ли?