LINUX.ORG.RU

Вопрос про архитектуру кода.

 , ,


0

1

есть у меня 4 функции.

Первая запрашивает ldap и отдает словарь.

Вторая запрашивает ldap, а так же использует словарь от первой функции и отдает словарь.

Третья запрашивает ldap, использует и редактирует (добавляет данные) словари от первых двух функций и тоже отдает словарь.

И, наконец, четвертая использует вообще все словари , чтобы составить итоговый словарь, который потом будет использоваться для шаблонов flask.

И поскольку все так перемешанно, то словари я делаю глобальными.

И я понимаю, что это очень рукожопно, но не понимаю, как сделать так, чтобы было красиво.

Пихать все в одну функцию?

★★★

Ответ на: комментарий от vvn_black

Цепочка вызовов функций, результат работы одной является аргументом следующей.

Вот не получается так. Я еще раз пересмотрел код, функции не изменяют словарей других функций, те легче.

Но

первая функция отдает словаь. этот словарь использует вторая функция. А четвертая испольует словарь и от второй и от первой.

Те я бы мог в четвертой вызвать первую и вторую , чтобы получить словари, но первую уже вызывали во второй, а лишний раз дергать ldap запросами я не хочу.

constin ★★★ ()
Ответ на: комментарий от anonymous

Поместите в vector, список, ... массив, дерево, ... ссылки на словари и передавайте в функции ссылку на vector, ...

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

constin ★★★ ()

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

anonymous ()
Ответ на: комментарий от anonymous

тут можно сказать что без кода можете закрыть тему и не заниматься словоблудием

да разве будет кто-то читать чужой код?

audit_table - итоговая функция. ей требуется словарь от get_group_members и get_shares.

но в тоже время get_shares тоже хочет словарь от get_group_members

def get_shares():

    shares_search_filter = "(&" \
                        "(objectClass=univentionShare)" \
                        "(univentionShareHost=fileserver*.domain.localnet)" \
                        "(!(univentionShareSambaName=tauschordner))" \
                        ")"
    shares_attr = ['dn', 'univentionShareSambaName',
                   'univentionFreeAttribute1',
                   'univentionFreeAttribute2',
                   'univentionShareSambaValidUsers']
    shares_search_base = 'cn=shares,dc=domain,dc=localnet'

    shares_ldap = connect.search_s(shares_search_base, ldap.SCOPE_SUBTREE,
                                   shares_search_filter, shares_attr)

    output = {}
    for key, value in shares_ldap:

        share_name = value.get('univentionShareSambaName', '')
        share_name = html_decode(share_name[0])

        groups = value.get('univentionShareSambaValidUsers', '')
        groups = [html_decode(groups) for groups in groups]
        # convert all words in one list to string
        groups = ''.join(groups)
        # convert string to separated list
        groups = groups.split()
        # remove '@' from groups names
        groups = [groups[1:] for groups in groups]
        owner1 = value.get('univentionFreeAttribute1', '')
        owner1 = html_decode(owner1[0])
        owner2 = value.get('univentionFreeAttribute2', '')
        owner2 = html_decode(owner2[0])
        dic = {share_name: [groups, owner1, owner2]}
        output.update(dic)
    #add export groups to shares dict
    for group, users in group_members.items():
        export = re.search("share_exporte_", group)
        if export:
            if group != "share_exporte_r":
                owner1 = users[1]
                owner2 = users[2]
                share = group[6:][:-2]

                dic = {share: [{group}, owner1, owner2]}
                output.update(dic)

    return output


def get_table_uid_usernames():

    ldap_querry = connect.search_s(
        'cn=users,dc=domain,dc=localnet',
        ldap.SCOPE_SUBTREE,
        'objectClass=organizationalPerson',
        ['uid','displayName', 'shadowExpire'])
    output = {}
    for key, value in ldap_querry:
        display_name = value.get('displayName', '')
        display_name = html_decode(display_name[0])
        uid = value.get('uid', '')
        uid = html_decode(uid[0])
        shadow_expire = value.get('shadowExpire', '')
        if shadow_expire:
            shadow_expire = shadow_expire[0].decode("utf-8")
            display_name = "{} *(gesperrt)".format(display_name)
        dic={uid: display_name}
        output.update(dic)
    return output




def get_group_members():

    members_search_filter = 'cn=share_*'
    members_attr = ['memberUid', 'cn', 'univentionFreeAttribute1',
                    'univentionFreeAttribute2']
    members_search_base ='cn=groups,dc=domain,dc=localnet'

    members_ldap = connect.search_s(members_search_base, ldap.SCOPE_SUBTREE,
                          members_search_filter, members_attr)
    output={}
    for key, value in members_ldap:
        group_name = value.get('cn', '')
        group_name = html_decode(group_name[0])

        member_uids = value.get('memberUid', '')
        member_uids = [html_decode(member_uids) for member_uids in member_uids]
        owner1 = value.get('univentionFreeAttribute1', '')
        if owner1:
            owner1 = html_decode(owner1[0])
        owner2 = value.get('univentionFreeAttribute2', '')
        if owner2:
            owner2 = html_decode(owner2[0])
        dic = { group_name: [member_uids, owner1, owner2]}
        output.update(dic)
    return output

def get_audit_table():

    output={}
    for key, value in sorted(shares.items()):
        group_owners = {key:
                    {'OWNER1': users_names[shares[key][1]],
                    'OWNER2': users_names[shares[key][2]],
                    'READ':[],
                    'WRITE':[]}
                    }
        output.update(group_owners)
        for group in value[0]:
            DisplayNames = []
            for uid in group_members[group][0]:
                DisplayNames.append(users_names[uid])
            rw = re.search("_rw$", group)
            if (rw):
                dic = {key: {'WRITE': DisplayNames}}
                update(output, dic)
            else:
                dic = {key: {'READ': DisplayNames}}
                update(output, dic)
        list_length = max(len(output[key]['READ']), len(output[key]['WRITE']))
        dic = {key: {'max_length': list_length}}
        update(output, dic)


    return output

def get_global():
    ldap_connection_init()
    global group_members
    global shares
    global users_names
    global audit_table
    group_members = get_group_members()
    shares = get_shares()
    users_names = get_table_uid_usernames()
    audit_table = get_audit_table()

constin ★★★ ()
Ответ на: комментарий от constin

Те я бы мог в четвертой вызвать первую и вторую , чтобы получить словари, но первую уже вызывали во второй, а лишний раз дергать ldap запросами я не хочу.

кто сказал «мемоизация»? ;)

aol ★★★★★ ()

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

bvn13 ★★★★★ ()
Ответ на: комментарий от constin

Те я бы мог в четвертой вызвать первую и вторую , чтобы получить словари, но первую уже вызывали во второй, а лишний раз дергать ldap запросами я не хочу.

Выдели функции которые ходят в лдап. И фунции, которые работают с результатами. Тогда

ya-betmen ★★★★★ ()
Ответ на: комментарий от anonymous

а в функции нынче не модно передавать нужные таблицы

кстати, да. можно попробвать.

оберни все в класс

я стараюсь избегать классы насколько это возможно.

constin ★★★ ()
Ответ на: комментарий от constin

я стараюсь избегать классы насколько это возможно

А причина? Классы, ну и ООП - это нормальный подход для изоляции данных. Ненормальный, это ФП. )

vvn_black ★★★★★ ()
Последнее исправление: vvn_black (всего исправлений: 1)
Ответ на: комментарий от anonymous

Вот так вроде отлично.


def get_audit_table():

    users_names = get_table_uid_usernames()
    group_members = get_group_members()
    # передаем полученный ранее словарь в функцию, как посоветовали выше
    shares = get_shares(group_members)
    output=.......
......



    return output
constin ★★★ ()
Ответ на: комментарий от vvn_black

А причина?

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

constin ★★★ ()

Не понял проблемы.

firstDict := foo()
secondDict := bar(firstDict)
thirdDict := baz(firstDict, secondDict)
quux(firstDict, secondDict, thirdDict)
Зачем нужны глобальные переменные?

Miguel ★★★★★ ()
Ответ на: комментарий от constin

Всё так, код большого и сложного проекта без ООП становится не поддерживаемым. У ФП свои ниши, не надо бездумно тащить его везде. Процедурный стиль себя изжил, он остался только там, где иначе никак. Есть ещё паттерны, SOLID и прочие best practices.

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

InterVi ★★ ()
Ответ на: комментарий от InterVi

Владимир

Можно в стиле ООП разработать любую программу и не понимающие когда нужны классы, когда процедурный код, ... так и делают.
Да еще говорят, что и архитектура проекта и реализация отличны.

А вообще категоричность в суждениях - не хорошо.

anonymous ()
Ответ на: комментарий от anonymous

Владимир

Добавочка.
Впрочем упускаю к примеру то, что имеется такое понятие - «стиль» программирования.
Может быть те кто используют ООП «везде и вся» в чем, то правы.
Но вот API библиотек /ИМХНО/ должно быть разработано «правильно».

Плохой код - «себе дороже».
Всегда стремлюсь к «Где просто, там ангелов со ста».

anonymous ()

И я понимаю, что это очень рукожопно, но не понимаю, как сделать так, чтобы было красиво.

Завернуть все в класс, как выше советовали. Это наиболее адекватный способ. Все кишки с парсингом надо вынести в приватные методы, иначе выглядит как лапша.

qaqa ()
Ответ на: комментарий от constin

Конечно. Короче смотри, мутишь фабрику, которая возвращает кешируемый синглтон, а в конструкторе передаешь di-аксессор для лдапа (реализуешь отдельно), с делегированием по получению ответов. Далее уже в колбеках на инстансе пишешь свой код по сборке конечного дикта. Три страницы болерплейта и вот ты уже почти пишешь первые строки программы.

Можно конечно просто модуль сделать с функцией, но это не так круто, плюс каждый сможет зайти и сразу поправить, а значит быстрее внести баги.

anonymous ()
Ответ на: комментарий от anonymous

Смехуечки-смехуечками, но весь этот оверинжениринг кажется таковым только пока не встаёт задача адекватно покрыть это дело тестами. И тут внезапно все плюсы и становятся явными.

qaqa ()
Ответ на: комментарий от constin

Эм, модули питона, к слову, синглтонами становатся в момент загрузки.

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

Так же непопулярное мнение о том что стоит поставить кеш для запросов в проде, redis/memcached например. Ты же только читаешь из ldap?

dtm ()
Ответ на: комментарий от qaqa

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

constin ★★★ ()