Suggestion for refactoring

[eugene-eeo]

Apologies if I sound a bit blocky and bot-like in my comment, it's like 12:33AM where I am. See MQLite.py:425, a conditional is not needed as you have already stated all of the possible conditions in the __init__ function. Also in line 438, not wanting to sound like a pendant it is generally not recommended to put spaces after equals sign in function arguments. For WrapConstraintAnd you may implement it as the following which would be more intuitive:

class WrapConstraintAnd(object):
    def __init__(self, *conds):
        self.conds = conds

    def match(self, data):
        return all(k(data) for k in self.conds)

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

class Compiler(object):
    # code here
    compilers = {bool: MatchEqual, dict: compile_dict, ...}

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

class Pattern(object):
    def __init__(self, data):
        self._compiler = Compiler()
        self._data = data
        self._compiled = None

    def compile(self):
        pattern = self._compiled = self._compiler.compile(self._data)
        return pattern

    def match(self, data):
        return self._compiled.match(data) if self._compiled else self.compile().match(data)

[Beluki]

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.

[eugene-eeo]

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