4
\$\begingroup\$

I run into issues where I forget to run UI changes on the main thread.

I've just fixed an issue for loading and unloading an activity indicator and fixed it with the following code:

if active {
    DispatchQueue.main.async {
        UIApplication.shared.keyWindow?.startIndicatingActivity()
    }
} else {
    DispatchQueue.main.async {
        UIApplication.shared.keyWindow?.stopIndicatingActivity()
    }
}

Now an alternative is this code:

DispatchQueue.main.async {
    if active { UIApplication.shared.keyWindow?.startIndicatingActivity()
    } else { UIApplication.shared.keyWindow?.stopIndicatingActivity() 
    }
}

Now which one is right / more maintainable for the future?

\$\endgroup\$
2
  • \$\begingroup\$ Where do the start/stopIndicatingActivity methods come from? \$\endgroup\$
    – Martin R
    Commented Sep 27, 2019 at 6:59
  • \$\begingroup\$ Given how little processing each takes to execute, I'd say it doesn't matter. Whichever way makes you happy. \$\endgroup\$ Commented Sep 28, 2019 at 0:55

2 Answers 2

5
\$\begingroup\$

From a processing efficiency perspective, they are largely indistinguishable. But if I were forced to choose between these two approaches, I might lean towards the latter, wrapping the whole if-else test in a single dispatch to the main queue, for two reasons:

  1. There is a timing issue:

    Let’s assume you ran this from a background thread and active was false, so you asynchronously dispatch the activity indicator update back to the main queue. What if, by the time the main queue gets to the dispatched block, that active is now true? Do you still really want to turn off the activity indicator? No, you almost certainly want to treat the combined test of active and the corresponding updating of the activity indicator as a single operation.

  2. There is a thread safety issue:

    When dealing with this boolean (and I’m assuming it’s a simple boolean, not wrapped in some synchronization mechanism), you should think about how you ensure thread-safe access to it. One really should synchronize access to one’s properties when writing multi-threaded code, and the first approach makes it unclear from which thread are you retrieving active (much less from which thread(s) is it being updated). One very simple synchronization technique to ensure that all reads and writes to active are done on the main queue, which, by virtue of being a serial queue, ensures thread-safety.


A few observations regarding other possible patterns/considerations:

  1. You might want to consider whether, rather than writing a routine that you manually call to check active and update the activity indicator accordingly (meaning that every time you update active, you have to remember to call this routine), whether you might prefer to just supply the active property an observer that does the updating of the activity indicator for you. If you can have your property automatically trigger the UI update, then it eliminates the possibility of them getting out of sync.

  2. There are special concerns if the changes to active are happening very quickly:

    • First, if active changes are happening extremely quickly (e.g. thousands of times per second), you risk flooding the main queue. You can address these sorts of issues with things like dispatch sources.

    • Second, even if it’s only a couple of times per second, you might want to avoid starting and stopping the activity indicator too quickly (because it yields a spinner that is constantly restarting, yielding a “Max Headroom” style of stuttering effect). For example, if active changes every ¼ second, and keeps doing that for 10 seconds, you may want to just want to keep the spinner going for that full 10 seconds, not starting and stopping it repeatedly. You can accomplish this by programming some latency in the “stop activity indicator” routine. E.g. when active is set to false, add non-repeating timer to stop the activity indicator in ½ second, but only after canceling any prior timer, if any. Likewise, when active is set to true, only “start” the activity indicator if you know it’s not already started.
       

    Bottom line, be aware that the right implementation might depend upon the frequency of changes.

\$\endgroup\$
2
\$\begingroup\$

If you want to do more things in the main thread when active is true or when it is false, then the first version would be recommendable.

Bear in mind that checking for active being true or false is done quicker in the first version since it's made on the main thread. In the second version, the checking isn't done until the background thread starts executing. By then, active may have changed in value from when the async call was made.

Here is a sample test that illustrates the behaviour of concurrent code:

var i = 0
for _ in 0..<10 {
    i += 1
    let str = "\(i)\t="
    DispatchQueue.main.async {
        print("Back", str, i)
    }
    print("Main", str, i)
}
print("------------")

It may print in the console:

Main 1    = 1
Main 2    = 2
Main 3    = 3
Main 4    = 4
Main 5    = 5
Main 6    = 6
Main 7    = 7
Main 8    = 8
Main 9    = 9
Main 10   = 10
------------
Back 1    = 10
Back 2    = 10
Back 3    = 10
Back 4    = 10
Back 5    = 10
Back 6    = 10
Back 7    = 10
Back 8    = 10
Back 9    = 10
Back 10   = 10

(The order of the lines that are printed asynchronously, may vary).

This is the difference between checking the value of a variable on the main thread or asynchronously.


If you are aiming for conciseness, here are a couple of versions that use Optional map and are similar in functionality to your versions respectively:

1)

UIApplication.shared.keyWindow.map { w in
    active?
        DispatchQueue.main.async { w.startIndicatingActivity() }
        :
        DispatchQueue.main.async { w.stopIndicatingActivity()  }
}

2)

DispatchQueue.main.async {
    UIApplication.shared.keyWindow.map {
        active ? $0.startIndicatingActivity() : $0.stopIndicatingActivity()
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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