1

I asked a similar question to this on SO, but after re-reading it this morning, I realized it wasn't clear what I'm asking. So I narrowed down my problem and realized it was about a pattern instead of a coding problem.

I found this question, but it's related to C# and Tasks, but it's along the same lines.

Suppose I'm using a thread to download a file with the option of cancelling:

import threading

class DownloadThread(threading.Thread):

    def __init__(self):
        self._keep_download_active = True

    def run(self):
        while self.keep_download_active):
            # code to download file

    def cancel(self):
        self._keep_download_active = false 

Python does have Event objects. According to the docs:

This is one of the simplest mechanisms for communication between threads: one thread signals an event and other threads wait for it

Much like the C# link stated about CancellationToken, Event is thread-safe. Which means I can write my code like this:

import threading

class DownloadThread(threading.Thread):

    def __init__(self):

        # threading.Event() is false by default
        self._stop_download = threading.Event()

    def run(self):
        while self._stop_download:
            # code to download file

    def cancel(self):
        self._stop_download.set()

Notice, in the first example I'm not passing a boolean into the constructor, I'm declaring it in the constructor, and it's kept private (_ indicates to the reader to treat it as private).

Given this current context, is doing so considered code smell? Would it be better to use what is provided by the language, in this case the second example?

If the boolean was passed into the constructor, then I can see it being a code smell, because then that variable might be shared across multiple threads and cause race conditions.

  • You can avoid a race condition by allowing one-time-only use of the cancel mechanism (the user can't un-cancel it, once it is cancelled). – Robert Harvey Jul 17 '18 at 15:26
  • @RobertHarvey - In that case I would check to see if it's already cancelled, if so do nothing. Correct? – user310745 Jul 17 '18 at 15:31
  • Something like that. – Robert Harvey Jul 17 '18 at 15:31
  • @RobertHarvey - So either way, the boolean or event is acceptable. Just ensure once it's been cancelled that nothing happens. – user310745 Jul 17 '18 at 15:32
  • I think you might be overthinking this. Cancelling your download is going to move execution to somewhere else that doesn't look at the cancellation variable anymore, so I don't even think you have a race condition in the first place. – Robert Harvey Jul 17 '18 at 15:35

Your Answer

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