Pergunta

To ilustrate my main concern let's start by considering a "trivial" typical problem, data filtering & parsing coming from a process and dumping the information onto something {gui console, file, stdout, ...}. Now, let's say we've got a common "codebase" like (minimal example and very simplified):

class Listener:

    def on_process(self, line):
        print(line)


class CommandExecutor:

    def __init__(self, on_process=None):
        self.on_process = on_process

    def run(self):
        lines = [
            ".\\a.py:9:9: F841 local variable 'foo' gives error1",
            ".\\b.py:9:9: F842 local variable 'bar' gives error2",
            ".\\c.py:9:9: F843 local variable 'foo' gives error3"
        ]

        for l in lines:
            if self.on_process:
                self.on_process(line=l)

So, after some thinking let's say I've picked up from the dozen possible solutions just 3, ie:

SOLUTION1

if __name__ == "__main__":
    l = Listener()

    def process_line(line):
        if "F842" in line:
            return False, None
        else:
            return True, f"{line} <---PARSED SUCCESFULLY"

    def on_process(**kwargs):
        line = kwargs["line"]
        valid_line, new_line = process_line(line)
        if valid_line:
            l.on_process(new_line)

    c = CommandExecutor(on_process)
    c.run()

SOLUTION2

if __name__ == "__main__":
    l = Listener()

    def process_line(line):
        if "F842" in line:
            raise Exception("Invalid line")
        else:
            return f"{line} <---PARSED SUCCESFULLY"

    def on_process(**kwargs):
        try:
            line = kwargs["line"]
            new_line = process_line(line)
            l.on_process(new_line)
        except Exception as e:
            pass

    c = CommandExecutor(on_process)
    c.run()

SOLUTION3

if __name__ == "__main__":
    l = Listener()

    def check_line(line):
        if "F842" in line:
            return False
        else:
            return True

    def parse_line(line):
        return f"{line} <---PARSED SUCCESFULLY"

    def on_process(**kwargs):
        line = kwargs["line"]
        if check_line(line):
            l.on_process(parse_line(line))

    c = CommandExecutor(on_process)
    c.run()

And after some thinking concluded that:

  • Solution1: It's the less pythonic one, possible the dirtiest one from the whole set but probably the "faster" (microoptimization)
  • Solution2: It's "cleaner" than solution1 but probably is doing too much, it doesn't have proper separation of concerns
  • Solution3: It's probably the cleanest code but it'll be quite verbose one and maybe doesn't give too many advantages

I'm quite interested to know whether there is an obvious "best candidate" from the above set of solutions... or if there is a better solution than the proposed ones.

Thing is, I tend to waste quite of time when it comes to choose a solution to solve a particular specific problem because usually I come up with a lot solutions... In fact, I've just posted 3 possible ways but in my head I've got few dozens... And that's really bad cos obviously it's a big waste of time and a big indication that I don't know the best way to face a particular problem. I like when I can choose a coding solution as inmmediatly as possible

So the question here would be, how to decide which abstract solution to stick with before starting using it widely on your app/s? You really want to avoid refactoring as little as possible when the number of apps using a particular solution has grown so in my book seems worth to give a fair ammount of thinking at the early stages before scaling up (I mean, extending your app by using inheritance, composition, plugins, ...).

Foi útil?

Solução

You're overthinking it. None of the solutions you've given us is any better than the others given the information we have. This code is small enough to not be worth your time to think about. If this code was in the innermost loop of a complex processing operation and you found it to be a performance bottleneck or source of bugs, then, and only then, should you worry about whether it's the optimal way to write it for your situation. If you found you need very similar code with minor differences in dozens of places in your code, it would be worth your time to see if refactoring it would make your code better. But until you know that this code is suboptimal – by measuring it – you shouldn't worry too much about such minor decisions.

Outras dicas

Why, and when to stop

Ask why you came up with a given solution. Corollary, what motivated you to seek another solution?

Off hand I have two reasons to seek alternatives. First to better convey meaning and intent. Second to conform to the overall design. No? It may be change for its own sake and it's time to stop.

Where's the architecture?

The difference between a boolean and an exception has profound implications and one or the other must be working against the larger design. Here then, is another signpost that it's time to stop.

Catching an exception upstream just to "pass" tells me there is no real point to it. Exceptions is not basic control structure. Also, in this case, it missed the opportunity to add context to the exception: where triggered and in the intervening layers as it bubbles up. These layers do have some meaning - context - don't they? Whether boolean or exception, it must done consistantly everywhere. Just like "consistently everywhere" is redundant so is the exception/boolean dichotomy. To muse between exception and a boolean this deep in the details tells me there a hole in the design.

Licenciado em: CC-BY-SA com atribuição
scroll top