search

HENNGE / aiodynamo

Asynchronous, fast, pythonic DynamoDB Client
https://aiodynamo.readthedocs.io/
Other
69 stars 20 forks source link

Flatten AND and OR conditions #173

Closed ojii closed 5 months ago

ojii commented 6 months ago

Previously, AND and OR conditions with more than two elements would lead to nested ANDs and ORs, making the resulting expression much more complicated and harder to read/understand. This change flattens AndCondition and OrCondition objects when they're combined.

Context: had to debug some complicated conditions (~10 AND's) and the extreme nesting makes it super hard to debug/read them.

~Not a huge fan of the name MinLen2AppendOnlyList so if someone has a better suggestion I'd love to hear it.~

dimaqq commented 6 months ago

Overall thoughts, sorry if imprecise.

I feel that the only use for the new class is the isinstance test. Everything else is a regular tuple. Would it be better to subclass a plain tuple instead?

And I managed to get it all wrong. Would it be better to change And/OrCondition's lhs/rhs to children which is a tuple (or a list)?

dimaqq commented 6 months ago

I've tried to express my thoughts on refactoring in #174, pull it in if you deem it worthy 🙇🏻

ojii commented 6 months ago

MinLen2AppendOnlyList needlessly generic, renamed to SubConditions and de-genericized.