-1

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, ...).

  • There are only two decisions to be made here: whether you want to use exceptions or error signalling, and whether you want two methods or just one. – Robert Harvey Jul 7 '18 at 14:58
  • @RobertHarvey That's quite interesting... as it takes me a lot of effort to forget about premature optimization when it comes to pick up a solution, even if I know for sure is bad. Also, I like that comment cos you've fastly evaluated which are the worthy decisions to consider... Usually in my head I tend to consider more decisions about the possible implications to "save" me time in future and as a result I end up wasting up quite a lot of time... Why have you considered only 2 decisions are worthy to solve the exposed problem? – BPL Jul 7 '18 at 15:08
  • Well, the rest of your code is essentially the same. – Robert Harvey Jul 7 '18 at 15:17
  • The main difference between 1&3 is how close the filter and the transformation are together. So if they're logically tightly coupled, prefer 1, and if they're loosely coupled (especially if you might want to compose the same filter with a different transform or vice-versa), prefer 2. Otherwise choose the one which is least typing and move on to the next problem :) – Useless Jul 7 '18 at 15:39
3

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.

  • Your answer contains some valid points, long time ago I'd heard the motto "Code Fearlessly, Refactor Mercilessly", guess your answer aligns with that practice. In any case, one of the reasons why I "invest" time trying to come up with the right pattern for a specific problem is because the next time I won't have to think which pattern to pick up anymore. So I'm not sure if coding fast and refactoring over and over each time you add more and more code till the code becomes cleaner and cleaner is worth in the long term. Sure, you get the job done much faster but... even so. Anyway, +1 – BPL Jul 7 '18 at 20:39
0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.