Добро пожаловать в форум, Guest  >>   Войти | Регистрация | Поиск | Правила | В избранное | Подписаться
Все форумы / PHP, Perl, Python Новый топик    Ответить
 Прошу совета в рефакторинге кода  [new]
Zhenek
Member

Откуда: Ванино
Сообщений: 499
Ситуация такая, есть у меня говно код, но не два дня в голову просто не лезет хорошая его реализация, все что получается сводится к одному.

Собственно ситуация:
$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 из правил. или региона или офиса или свои или все. но в будущем может быть и и то и то (например пользователь переведен в другой офис, но должен довести свой договор до конца, но данный вариант пока тут не рассматриваем)

 if (get_rules('edit_region_client') == true AND $info['region'] != $CUR_USER['region']) {
                                $die = 1;
                            } else {
                                $die = 0;
                                if (get_rules('edit_office_client') == true AND $info['office'] != $CUR_USER['office']) {
                                    $die = 1;
                                } else {
                                    $die = 0;
                                    if (get_rules('edit_my_client') == true AND $info['user'] != $CUR_USER['id']) {
                                        $die = 1;
                                    } else {
                                        $die = 0;
                                    }
                                }
                            }
5 окт 17, 16:13    [20845847]     Ответить | Цитировать Сообщить модератору
 Re: Прошу совета в рефакторинге кода  [new]
Hett
Member

Откуда: Бийск, Новосибирск
Сообщений: 12679
А взять нормальный фреймворк никак?
5 окт 17, 16:20    [20845873]     Ответить | Цитировать Сообщить модератору
 Re: Прошу совета в рефакторинге кода  [new]
Zhenek
Member

Откуда: Ванино
Сообщений: 499
автор
А взять нормальный фреймворк никак?


И правда) Чего это я) Более 70 файлов, 100 тысяч строк кода) Перепишу это на выходных) Люблю не оплачиваемую работу)))


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

Можете подсказать в каком фреймворке по вашему это реализовано более грамотно, а на досуге поковыряю его код и разберусь
5 окт 17, 16:32    [20845915]     Ответить | Цитировать Сообщить модератору
 Re: Прошу совета в рефакторинге кода  [new]
Hett
Member

Откуда: Бийск, Новосибирск
Сообщений: 12679
Тут форум по пхп, а не по телепатии. Никто не знает сколько кода там у вас и что за проект вообще.
Не обязательно взять и за раз все перенести, тот же symfony состоит из множества библиотек, которые можно внедрять себе и по одной, а со временем мигрировать полностью.
5 окт 17, 17:46    [20846163]     Ответить | Цитировать Сообщить модератору
 Re: Прошу совета в рефакторинге кода  [new]
Hett
Member

Откуда: Бийск, Новосибирск
Сообщений: 12679
смею предположить, что при грамотном подходе и использовании подходящих инструментов, код сократился бы в 5-10 раз. Тот кусок кода, что Вы привели, яркий пример, как вы расписали 3 условия на 20 строк.
5 окт 17, 17:49    [20846171]     Ответить | Цитировать Сообщить модератору
 Re: Прошу совета в рефакторинге кода  [new]
Zhenek
Member

Откуда: Ванино
Сообщений: 499
автор
Тут форум по пхп, а не по телепатии. Никто не знает сколько кода там у вас и что за проект вообще.
Не обязательно взять и за раз все перенести, тот же symfony состоит из множества библиотек, которые можно внедрять себе и по одной, а со временем мигрировать полностью.


Так я и не просил совета как мне переписывать и организовывать проект который еще и начинал делать не я, структуру определял не я.
Я попросил совета абстрагированно именно по одному куску, считайте это академической задачей уровня 9 класс.
20 строк я могу сократить сохранив текущий подход, но я специально разложил код, чтобы те, кто будет его смотреть не ломали голову.

Так-то он вообще выглядит вот так:

   
  $access = $user->check_group_rules('client'); //
        if ($access == 1) {
            access();
        }

и то что расписано в этой функции я могу игнорировать т.к. написано раз и в коде не мешается. Так я могу проверить любую группу прав на создание договора,пользователя,склада меняя edit_contract add_contract т.к. все правила имеют четкую структуру названий. а в функции уже будет get_rules('edit_region_'.$name)

Модератор:
Флуд удален.
Задавайте вопросы более конкретно, пожалуйста, чтобы потом не обижаться на ответы "не в тему".
--
vkle
5 окт 17, 18:14    [20846233]     Ответить | Цитировать Сообщить модератору
 Re: Прошу совета в рефакторинге кода  [new]
vkle
Member

Откуда: Самара
Сообщений: 13568
Zhenek
в true может быть выставлено только 1 из правил
switch...case именно так работает. Не?


Zhenek
пользователь переведен в другой офис, но должен довести свой договор до конца
А это уже другого толка условие, основанное на анализе свойств "владелец", "выполнено" и т.п. конкретного объекта (или экземпляра сущности) "договор".
5 окт 17, 18:50    [20846303]     Ответить | Цитировать Сообщить модератору
 Re: Прошу совета в рефакторинге кода  [new]
Zhenek
Member

Откуда: Ванино
Сообщений: 499
автор
switch...case именно так работает. Не?

Может я не понимаю, но если бы при запросе get_rules('edit_client') приходил бы ответ my region office или all
то да, я бы сделал так:

switch (get_rules(edit_client)) {
    case 'all':
       //
        break;
    case 'region':
        //
        break;
    case 'office:
        //
        break;
    case 'my:
        //
        break;
}


Но мне то нужно выполнить 4 проверки get_rules('edit_all_client') get_rules('edit_office_client') и т.д.
и помимо этого если он может редактировать клиентов своего офиса проверить а текущий клиент точно из его офиса. получается 3 проверки в каждой по 2 условия.

Я проверяю от больших возможностей к меньшим. сначала all потом region office my.


автор
Задавайте вопросы более конкретно, пожалуйста, чтобы потом не обижаться на ответы "не в тему"

ув. vkle куда же более конкретно то) Написан код и просьба его помочь отрефакторить. Расписаны переменные и функции. я лишь пытался увести разговор обратно в русло рефаткоринга конкретного куска
6 окт 17, 03:47    [20846947]     Ответить | Цитировать Сообщить модератору
 Re: Прошу совета в рефакторинге кода  [new]
vkle
Member

Откуда: Самара
Сообщений: 13568
Zhenek,

Не совсем так. Это лишь замена лесенке из if...else в данном куске кода.
switch...case проверяет каждое выражение case на соответствие с заданным в switch, начиная с первого. По первому true в сравнении выполняет блок кода и на этом прекращает работу. Таким образом, проверок может быть более одной. Блок кода отработает только на успешной проверке.
Получается вроде такого.
switch (true) {
  case (get_rules('edit_region_client') AND $info['region'] != $CUR_USER['region']) :
  //
  break;
  case (get_rules('edit_office_client') AND $info['office'] != $CUR_USER['office']) :
  //
  break;
  // и так далее
}

Фрагмент "== true" здесь мне представляется излишним, так как get_rules() уже возвращает true или false.

Однако, Вы верно заметили, следует ли всякий раз делать проверку get_rules() или можно один раз получить уровень привилегий один раз. На мой взгляд, простой уровень привилегий полезен в системах с относительно простой иерархией правил, в более сложной иерархии польза от него представляется сомнительной.
6 окт 17, 08:12    [20847027]     Ответить | Цитировать Сообщить модератору
 Re: Прошу совета в рефакторинге кода  [new]
Zhenek
Member

Откуда: Ванино
Сообщений: 499
Спасибо за ответ. Тогда пройду по всем участкам с ветвлениями if else и переделаю на switch case.
Буквально вчера после вашего ответа открывал документацию и не увидел там возможность вложенных условий в case, и только сейчас пролистав половину примеров пользователей, нашел его.

автор
Однако, Вы верно заметили, следует ли всякий раз делать проверку get_rules()


да, мне этот момент тоже не нравится, хотя бы потому, что каждый вызов get_rules делает запрос к бд, и на одной странице может быть до 5 проверок на права, и каждая по 4 запроса(( не считая рабочих выборок, выходит до 20 запросов на страницу, что нереально много.

Думаю делать 1 запрос на выбор всех прав в начале загрузки и возможно даже запихать его в сессию.
6 окт 17, 09:53    [20847200]     Ответить | Цитировать Сообщить модератору
 Re: Прошу совета в рефакторинге кода  [new]
tip78
Member

Откуда: Москва
Сообщений: 473
Zhenek
автор
Однако, Вы верно заметили, следует ли всякий раз делать проверку get_rules()


да, мне этот момент тоже не нравится, хотя бы потому, что каждый вызов get_rules делает запрос к бд, и на одной странице может быть до 5 проверок на права, и каждая по 4 запроса(( не считая рабочих выборок, выходит до 20 запросов на страницу, что нереально много.

Думаю делать 1 запрос на выбор всех прав в начале загрузки и возможно даже запихать его в сессию.

в ф-ю первой строкой: static $arr;
в $arr 1м запросом пихнуть все права (возможно битмапом)
юзать $arr все остальные заходы в ф-ю
PROFIT
6 окт 17, 12:22    [20847770]     Ответить | Цитировать Сообщить модератору
 Re: Прошу совета в рефакторинге кода  [new]
Sidmal
Member

Откуда:
Сообщений: 242
Zhenek, я бы написал как-нибудь так:

/**
 * @return array
 **/
function get_rules()
{
    //...

    return ['edit_region_client' => true, 'edit_office_client' => false, 'edit_my_client' => true];
}

/**
* @param array $info
* @param array $CUR_USER
* @return bool
**/
function isGrant(array $info, array $CUR_USER)
{
    $rules = get_rules();

    $mapping = [
        ['rule' => 'edit_region_client', 'info' => 'region', 'user' => 'region'],
        ['rule' => 'edit_office_client', 'info' => 'office', 'user' => 'office'],
        ['rule' => 'edit_my_client', 'info' => 'user', 'user' => 'id']
    ];

    foreach ($mapping as $map)
    {
        if (!isset($rules[$map['rule']]) || false === $rules[$map['rule']]
            || $info[$map['info']] == $CUR_USER[$map['info']]) {
            continue;
        }

        return true;
    }

    return false;
}

$die = 0;
$info = [];
$CUR_USER = [];

if (true === isGrant($info, $CUR_USER)) {
    echo 'что-то делаем!';
}


за работоспособность кода не ручаюсь, писал исключительно в качестве примера, чтобы показать основную концепцию.
т.е. основная прелесть такого подхода в том, что:
  • экономим на списках, делая только один вызов метода get_rules
  • мы выносим правила валидации доступа в отдельный настроечный массив $mapping, добавляя коду гибкости, в случае добавления стандарных правил валидации код не надо переписывать, надо только дополнить массив. а если например данный массив поместить на сервер как json структуру, то даже в код лазить не придется.
  • 6 окт 17, 12:54    [20847938]     Ответить | Цитировать Сообщить модератору
     Re: Прошу совета в рефакторинге кода  [new]
    Hett
    Member

    Откуда: Бийск, Новосибирск
    Сообщений: 12679
    Вот уже дошли до того, что создали свою свой инструмент для работы с правами. Еще немножко и можно будет готовый взять.
    6 окт 17, 13:16    [20848033]     Ответить | Цитировать Сообщить модератору
     Re: Прошу совета в рефакторинге кода  [new]
    Sidmal
    Member

    Откуда:
    Сообщений: 242
    Hett
    Вот уже дошли до того, что создали свою свой инструмент для работы с правами. Еще немножко и можно будет готовый взять.


    Я с вами полностью согласен, в принципе задача автора решается прекрасно Symfony Security Component + кастомные voters к нему.
    Но автор по ходу переписки дал понять, что не готов в данный момент поменять кодовую базу, поэтому предлагается решение в рамках указанны условий.
    6 окт 17, 13:56    [20848260]     Ответить | Цитировать Сообщить модератору
     Re: Прошу совета в рефакторинге кода  [new]
    Zhenek
    Member

    Откуда: Ванино
    Сообщений: 499
    автор
    Вот уже дошли до того, что создали свою свой инструмент для работы с правами. Еще немножко и можно будет готовый взять.


    Так я Вас попросил еще вчера, скажите название инструмента, названия фреимворка и название функции например для работы с правами, дальше я нагуглю)))
    6 окт 17, 14:21    [20848393]     Ответить | Цитировать Сообщить модератору
    Все форумы / PHP, Perl, Python Ответить