amaslyaev / noorm

NoORM (Not only ORM) - Python library that makes your database operations convenient and natural
MIT License
16 stars 0 forks source link

Simplify DB-API functions that just transfer input arguments to query parameters #8

Closed amaslyaev closed 2 weeks ago

amaslyaev commented 1 month ago

In many cases DB-API functions look like this (for positional parameters):

@nm.sql_fetch_all(
    DbOrder, "select id, number, date, user_id, amount from orders where user_id = ? and date >= ?"
)
def get_user_orders(user_id: int, min_date: date):
    return nm.params(user_id, min_date)

or like this (for named parameters):

@nm.sql_fetch_all(
    DbOrder, "select id, number, date, user_id, amount from orders where user_id = :user_id and date >= :min_date"
)
def get_user_orders(user_id: int, min_date: date):
    return nm.params(user_id=user_id, min_date=min_date)

To reduce ugly-looking and error-prone repetition it would be helpful to have possibility to say "pass all function arguments as positional SQL parameters" or "as named parameters". Examples:

@nm.sql_fetch_all(
    DbOrder, "select id, number, date, user_id, amount from orders where user_id = ? and date >= ?"
)
def get_user_orders(user_id: int, min_date: date):
    return nm.PARAMS_APPLY_POSITIONAL

For named parameters:

@nm.sql_fetch_all(
    DbOrder, "select id, number, date, user_id, amount from orders where user_id = :user_id and date >= :min_date"
)
def get_user_orders(user_id: int, min_date: date):
    return nm.PARAMS_APPLY_NAMED

This enhancement is inspired by this discussion: #6

LecronRu commented 3 weeks ago

Когда-то вы писали:

Не знаю как у кого, но у меня сразу в мозгу загорается красная лампочка "Ошибка!". Функция пообещала вернуть список интеджеров, а возвращает None. Мне нужно поднять глаза на строчку выше, увидеть декоратор, вспомнить, что этот декоратор меняет сигнатуру, и поэтому list[int] это не враньё, а очень важная инструкция для @some_decorator. Пока я на это всё отвлекался, уже забыл, зачем забрёл в этот кусок кода, потерял мысль, выпал из потока.

Возврат константы в этом контексте выглядит еще хуже. Возможно следует заменить на функции. params_by_pos и params_by_name, в дополнение к простой params. Или еще лучше, params_auto(mode=BY_NAME|BY_POS) Если учесть что позиционные статистически более популярны, BY_POS можно сделать значением по умолчанию

amaslyaev commented 3 weeks ago

А вот здесь лампочка не загорается. У функции get_user_orders не прописано, что она возвращает. Соответственно,

Почему возврат констант мне нравится больше вызова функции. Когда мы видим вызов функции, мы подсознательно предполагаем, что эта функция что-то делает. А тут нам ничего делать не надо. Только подать сигнал, что надо применить некий умолчальный сценарий – законсьюмить аргументы функции в параметры запроса. Просто сигнал. Спецконстанты для этого вполне годятся.

Вообще, если интересно, ожидаемая декоратором сигнатура DB-API функции какая-то такая:

@nm.sql_fetch_all(DbSomething, "select ...")
def get_something(...) -> PrepareFuncResult | ParamsAutoEnum | None:
    # ... some code

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

LecronRu commented 2 weeks ago

У функции get_user_orders не прописано, что она возвращает.

Это если не прописано. Но для удобства работы в IDE и документирования кода — будут прописывать. Причем не PrepareFuncResult | ParamsAutoEnum | None, а реальные возвращаемые из базы данные.

@nm.sql_fetch_all(DbSomething, "select ...")
def get_something(...) -> list[DbSomething]:
    # ... some code

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

amaslyaev commented 2 weeks ago

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

amaslyaev commented 2 weeks ago

Done in version 0.1.6.

LecronRu commented 1 week ago

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

Так библиотека — архитектурная ошибка. Функция не должна знать о способах ее использования, в том числе декорирования. Понимал это с самого начала. Но молчал, так как удобно. Но когда баг начинают выдавать за фичу, укреплять эту ошибку, считаю нужным обратить внимание. При PrepareFuncResult | ParamsAutoEnum | None, функция должна называться prepare_to_decorator_for_get_something. Потому что просто get_something должна возвращать something, который мы get.

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

И снова обращаю внимание на FastAPI. При указании response_model, в аннотации возврата разработчик может указать что угодно. Точнее, что удобно. Впрочем, это не ключевое замечание, а обсуждение когнитивного диссонанса. О котором вы заговорили первым. Так или иначе он будет. Главное, с каким из них проще работать пользователю библиотеки, а не ее сздателю.