Suggestion for refactoring about mqlite HOT 2 CLOSED

Hi there! Thanks for the suggestions.

MQLite.py:425, a conditional is not needed as you have already stated all of the possible conditions in the init function.

The self.order == 'reverse': test isn't needed, it's there to avoid forgetting it in case I add more order options.

it is generally not recommended to put spaces after equals sign

The = spaces are a stylistic choice. I always put spaces around keyword arguments in all my python code (even if that's not what pep8 suggests I find it more readable).

WrapConstraintsAnd has two arguments because of performance. It's unusual to have single keys with more than two constraints so I optimize for the common case, avoiding a loop at runtime when possible. There are actually two places when all/any would be reasonable though, ConstraintSuffixAll and ConstraintSuffixAny.

The long block of conditionals at line 529 may also be refactored into a dict:

In it's current form, yes. I didn't do it because I was thinking about the possibility of adding compiler options such as comparing string in a case-insensitive way. That would have to be added here:

if isinstance(pattern, str):
    if 'case-insensitive' in self.options:
        do_something()
    else:
        do_another_thing()

The rationale is that if an user creates a subclass of the Compiler and overrides compile_str, the options are still honored.

for caching classes like Pattern I usually implement the caching like the following:

The idea about Pattern and JSONPattern is that the user doesn't need to care about the internals, thus I don't return the compiled pattern in the compile() method.

But seriously, thanks a lot for your suggestions, I appreciate them. I'm pretty sure I sound waaaay more pedantic than you with all those explanations. Sorry for that, I'm incredibly picky about my style choices. I hope that at least I don't sound like a complete jerk that refuses every advice no matter what.

from mqlite.

👍 understood, just two different approaches to the same problem IMHO.

from mqlite.