Как вы вообще относитесь к коду из разряда is_valid: bool =

....
if not is_valid:
return False


Норм это или не очень по питоновски?

зависит от того насколько сложен этот is_valid

Чем именно этот код примечателен? Ну, кроме избыточного тайпхинта.

Потому что меня всегда смущал код из разряда if True: return True

То что is_valid True всегда или фолс всегда

покажи функцию целиком вместе с названием

Э... Ты показал разные вещи. Вообще я бы посчитал подобное подозрительным и скорее всего писал бы return True, а не return is_valid.

Как понимаю вопрос в том, не лучше ли писать is_valid: bool = .... if not is_valid: return is_valid Имхо, с явным False сильно читаемее, а здесь повтор застявляет задуматься

Просто тогда большой вопрос зачем нам is_valid, если тру по сути и есть ис валид

А если сравнивать с кейсом когда вообще не стоит его писать?. is_valid = True if asked_fields and config.mask_fields: for mask_to_exec in config.mask_fields: ... is_valid = True if not asked_fields or not config.mask_fields: return is_valid for mask_to_exec in config.mask_fields: ....

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

а зачем тут вообще флаг?

Там сложнее логика после 3 точек

https://t.me/ru_python_beginners/2761526

Если is_valid - просто чтоб не передавать литерал в рамках функции, то это, конечно, странное решение

И если в if true: return true не подразумевается какая то переменная, которая может быть true, то дело даже не в стиле

вот вся функция где есть исвалид https://pastebin.com/vmmDJR06

ух е

А не легче Exception плюнуть?

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

Я б выкинул все is_valid и делал py if not some_condition: return False .... return True

21 и 22 строки вообще висят невесть где и выглядят как потенциальный баг.

https://pastebin.com/Wt2pryRy над названиями только поработай

почему разный? ошибка ж одинаковая

М... Первая функция потенциально возвращает None...

Разумно, да

это ранний ретурн, то есть если фолс то сразу выходим

Что за дерьмо

Это какой-то поздний и внезапный ретурн. Ранний привязан непосредственно к проверке условия, а тут он где-то внезапно болтается. Скорее всего после десятой строчки должен был быть, но код пошевелили и он уплыл. В общем ты смешал подход с флагом и early return, полученная каша читается хуже.

Если оно в конфиге то почему оно дикт но может и не дикт??

да, это бы на этапе персинга бы унифицировать

Валидации конфига нет, видимо

потому что эта функция может применятся как например к филдам там и фильтрам

а зачем разделять это? if not asked_fields: return True if not config.mask_fields: return True

хуйня, дели на две

потому что я вообще не понял что за странные ситуации, я бы ещё комментарий добавил зачем эти проверки

да для такого некоторые умудряются целую иерархию классов сделать, а ты двух функций боишься :D

для вложенных филдов и обычных

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

а почему бы вместо if not all( check_mask_field(mask_element, users_permissions) for mask_element in mask_elements if mask_element.name in nested_asked_fields ): return False не сделать return not all()

Наверно можно

а насколько есть смысл писать else: raise TypeError вместо варианта без елс. Надо ли по конвенции не рейзить ошибку без условия?

я ориентируюсь на такую логику: если ветки условий равноправны по смыслу и особенно если их больше двух: писать else если у нас early return - то есть осоебнный кейс, то else не пишем

Я бы тут на этот else смотрел не как на равноправный, а как на "если дошли до конца функции, валидный случай не обнаружили".

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