|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
Ситуация такая, есть у меня говно код, но не два дня в голову просто не лезет хорошая его реализация, все что получается сводится к одному. Собственно ситуация: $CUR_USER - Массив содержит информацию о текущем авторизованном пользователе. Нужные нам элементы ['id'] ['region'] ['office'] соотв регион пользователя и ид офиса функция get_rules('edit_region_client') - проверяет может ли данный пользователь что-то делать. нужные нам параметры. возвращает true или false соотв. может/не может 'edit_all_client' - редактировать всех клиентов 'edit_region_client' - редактировать клиентов своего региона 'edit_office_client' - клиентов только своего офиса 'edit_my_client' - только своих клиентов $info - массив с данными открытого клиента. так-же ['user'] ['region'] ['office'] На данный момент реализовал так, срочность пропала, пришла пора привести в порядок. Но как сделать проврку более оптимальной + я думаю, что в такой реализации могут быть ложные срабатывания если ветвление пойдет не по той ветке. В теории в true может быть выставлено только 1 из правил. или региона или офиса или свои или все. но в будущем может быть и и то и то (например пользователь переведен в другой офис, но должен довести свой договор до конца, но данный вариант пока тут не рассматриваем) Код: php 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. 11. 12. 13. 14. 15.
... |
|||
:
Нравится:
Не нравится:
|
|||
05.10.2017, 16:13 |
|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
А взять нормальный фреймворк никак? ... |
|||
:
Нравится:
Не нравится:
|
|||
05.10.2017, 16:20 |
|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
авторА взять нормальный фреймворк никак? И правда) Чего это я) Более 70 файлов, 100 тысяч строк кода) Перепишу это на выходных) Люблю не оплачиваемую работу))) А по делу, мне все-же интересно помимо оставления за собой более менее красивого кода, оставить чуточку знаний в голове у себя, разобраться как сделать лучше, и возможно переписать весь кусок кода, отвечающий за распределение прав. Но сделать это самому, чтобы немного прокачать мой скилл))) Можете подсказать в каком фреймворке по вашему это реализовано более грамотно, а на досуге поковыряю его код и разберусь ... |
|||
:
Нравится:
Не нравится:
|
|||
05.10.2017, 16:32 |
|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
Тут форум по пхп, а не по телепатии. Никто не знает сколько кода там у вас и что за проект вообще. Не обязательно взять и за раз все перенести, тот же symfony состоит из множества библиотек, которые можно внедрять себе и по одной, а со временем мигрировать полностью. ... |
|||
:
Нравится:
Не нравится:
|
|||
05.10.2017, 17:46 |
|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
смею предположить, что при грамотном подходе и использовании подходящих инструментов, код сократился бы в 5-10 раз. Тот кусок кода, что Вы привели, яркий пример, как вы расписали 3 условия на 20 строк. ... |
|||
:
Нравится:
Не нравится:
|
|||
05.10.2017, 17:49 |
|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
авторТут форум по пхп, а не по телепатии. Никто не знает сколько кода там у вас и что за проект вообще. Не обязательно взять и за раз все перенести, тот же symfony состоит из множества библиотек, которые можно внедрять себе и по одной, а со временем мигрировать полностью. Так я и не просил совета как мне переписывать и организовывать проект который еще и начинал делать не я, структуру определял не я. Я попросил совета абстрагированно именно по одному куску , считайте это академической задачей уровня 9 класс. 20 строк я могу сократить сохранив текущий подход, но я специально разложил код, чтобы те, кто будет его смотреть не ломали голову. Так-то он вообще выглядит вот так: Код: php 1. 2. 3. 4. 5.
и то что расписано в этой функции я могу игнорировать т.к. написано раз и в коде не мешается. Так я могу проверить любую группу прав на создание договора,пользователя,склада меняя edit_contract add_contract т.к. все правила имеют четкую структуру названий. а в функции уже будет get_rules('edit_region_'.$name) Модератор: Флуд удален. Задавайте вопросы более конкретно, пожалуйста, чтобы потом не обижаться на ответы "не в тему". -- vkle ... |
|||
:
Нравится:
Не нравится:
|
|||
05.10.2017, 18:14 |
|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
Zhenekв true может быть выставлено только 1 из правилswitch...case именно так работает. Не? Zhenekпользователь переведен в другой офис, но должен довести свой договор до концаА это уже другого толка условие, основанное на анализе свойств "владелец", "выполнено" и т.п. конкретного объекта (или экземпляра сущности) "договор". ... |
|||
:
Нравится:
Не нравится:
|
|||
05.10.2017, 18:50 |
|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
авторswitch...case именно так работает. Не? Может я не понимаю, но если бы при запросе get_rules('edit_client') приходил бы ответ my region office или all то да, я бы сделал так: Код: php 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. 11. 12. 13. 14.
Но мне то нужно выполнить 4 проверки get_rules('edit_all_client') get_rules('edit_office_client') и т.д. и помимо этого если он может редактировать клиентов своего офиса проверить а текущий клиент точно из его офиса. получается 3 проверки в каждой по 2 условия. Я проверяю от больших возможностей к меньшим. сначала all потом region office my. авторЗадавайте вопросы более конкретно, пожалуйста, чтобы потом не обижаться на ответы "не в тему" ув. vkle куда же более конкретно то) Написан код и просьба его помочь отрефакторить. Расписаны переменные и функции. я лишь пытался увести разговор обратно в русло рефаткоринга конкретного куска ... |
|||
:
Нравится:
Не нравится:
|
|||
06.10.2017, 03:47 |
|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
Zhenek, Не совсем так. Это лишь замена лесенке из if...else в данном куске кода. switch...case проверяет каждое выражение case на соответствие с заданным в switch, начиная с первого. По первому true в сравнении выполняет блок кода и на этом прекращает работу. Таким образом, проверок может быть более одной. Блок кода отработает только на успешной проверке. Получается вроде такого. Код: php 1. 2. 3. 4. 5. 6. 7. 8. 9.
Фрагмент "== true" здесь мне представляется излишним, так как get_rules() уже возвращает true или false. Однако, Вы верно заметили, следует ли всякий раз делать проверку get_rules() или можно один раз получить уровень привилегий один раз. На мой взгляд, простой уровень привилегий полезен в системах с относительно простой иерархией правил, в более сложной иерархии польза от него представляется сомнительной. ... |
|||
:
Нравится:
Не нравится:
|
|||
06.10.2017, 08:12 |
|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
Спасибо за ответ. Тогда пройду по всем участкам с ветвлениями if else и переделаю на switch case. Буквально вчера после вашего ответа открывал документацию и не увидел там возможность вложенных условий в case, и только сейчас пролистав половину примеров пользователей, нашел его. авторОднако, Вы верно заметили, следует ли всякий раз делать проверку get_rules() да, мне этот момент тоже не нравится, хотя бы потому, что каждый вызов get_rules делает запрос к бд, и на одной странице может быть до 5 проверок на права, и каждая по 4 запроса(( не считая рабочих выборок, выходит до 20 запросов на страницу, что нереально много. Думаю делать 1 запрос на выбор всех прав в начале загрузки и возможно даже запихать его в сессию. ... |
|||
:
Нравится:
Не нравится:
|
|||
06.10.2017, 09:53 |
|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
ZhenekавторОднако, Вы верно заметили, следует ли всякий раз делать проверку get_rules() да, мне этот момент тоже не нравится, хотя бы потому, что каждый вызов get_rules делает запрос к бд, и на одной странице может быть до 5 проверок на права, и каждая по 4 запроса(( не считая рабочих выборок, выходит до 20 запросов на страницу, что нереально много. Думаю делать 1 запрос на выбор всех прав в начале загрузки и возможно даже запихать его в сессию. в ф-ю первой строкой: static $arr; в $arr 1м запросом пихнуть все права (возможно битмапом) юзать $arr все остальные заходы в ф-ю PROFIT ... |
|||
:
Нравится:
Не нравится:
|
|||
06.10.2017, 12:22 |
|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
Zhenek, я бы написал как-нибудь так: Код: php 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. 11. 12. 13. 14. 15. 16. 17. 18. 19. 20. 21. 22. 23. 24. 25. 26. 27. 28. 29. 30. 31. 32. 33. 34. 35. 36. 37. 38. 39. 40. 41. 42. 43. 44. 45.
за работоспособность кода не ручаюсь, писал исключительно в качестве примера, чтобы показать основную концепцию. т.е. основная прелесть такого подхода в том, что: экономим на списках, делая только один вызов метода get_rules мы выносим правила валидации доступа в отдельный настроечный массив $mapping, добавляя коду гибкости, в случае добавления стандарных правил валидации код не надо переписывать, надо только дополнить массив. а если например данный массив поместить на сервер как json структуру, то даже в код лазить не придется. ... |
|||
:
Нравится:
Не нравится:
|
|||
06.10.2017, 12:54 |
|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
Вот уже дошли до того, что создали свою свой инструмент для работы с правами. Еще немножко и можно будет готовый взять. ... |
|||
:
Нравится:
Не нравится:
|
|||
06.10.2017, 13:16 |
|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
HettВот уже дошли до того, что создали свою свой инструмент для работы с правами. Еще немножко и можно будет готовый взять. Я с вами полностью согласен, в принципе задача автора решается прекрасно Symfony Security Component + кастомные voters к нему. Но автор по ходу переписки дал понять, что не готов в данный момент поменять кодовую базу, поэтому предлагается решение в рамках указанны условий. ... |
|||
:
Нравится:
Не нравится:
|
|||
06.10.2017, 13:56 |
|
Прошу совета в рефакторинге кода
|
|||
---|---|---|---|
#18+
авторВот уже дошли до того, что создали свою свой инструмент для работы с правами. Еще немножко и можно будет готовый взять. Так я Вас попросил еще вчера, скажите название инструмента, названия фреимворка и название функции например для работы с правами, дальше я нагуглю))) ... |
|||
:
Нравится:
Не нравится:
|
|||
06.10.2017, 14:21 |
|
|
start [/forum/topic.php?fid=23&fpage=29&tid=1460497]: |
0ms |
get settings: |
9ms |
get forum list: |
13ms |
check forum access: |
4ms |
check topic access: |
4ms |
track hit: |
71ms |
get topic data: |
11ms |
get forum data: |
2ms |
get page messages: |
65ms |
get tp. blocked users: |
2ms |
others: | 309ms |
total: | 490ms |
0 / 0 |