Hello there, my sweet fellow developers,
Recently I stumbled upon a gem among pieces of ~sh…~ code.
Let me introduce you to the most awesome code in all of its glory. You’ve seen it in many different faces, but it always boils down to something like:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 |
let button = UIButton() let view = UIView() func badOnSwitch(sender: UISwitch) { if sender.isOn { view.isUserInteractionEnabled = true button.isUserInteractionEnabled = true } else { view.isUserInteractionEnabled = false button.isUserInteractionEnabled = false } } |
It’s too early, wipe the sweat, even if you didn’t write such code. You shouldn’t consider the types and methods, just the idea of the code. And the idea is pretty simple. You set the same value into multiple properties or methods of different objects abiding the same interface.
First of all, the code is counter DRY and it just doesn’t use the Bool as Bool. There is a lot of duplication in here as well. So, a better way would be to refactor the code like this:
|
1 2 3 4 5 |
func betterThanBadOnSwitch(sender: UISwitch) { view.isUserInteractionEnabled = sender.isOn button.isUserInteractionEnabled = sender.isOn } |
Right? Right? Wrong! We have deduplicated calls to isUserInteractionEnabled for same variables, but there are still 2 calls to isUserInteractionEnabled and there are two calls to isOn. Calling any property more than once in the scope of one function is a flaw by itself. Even, if your property is stored with potentially minimal overhead during access, it could change in the multithreaded environment, or it could become observable with some sophisticated observation code or even computed with some difficult computations. So, the best guideline is to never access the property or call the method more than once in a function, by properly decomposing the code and caching the method/property results:
|
1 2 3 4 5 6 |
func averageOnSwitch(sender: UISwitch) { let enabled = sender.isOn view.isUserInteractionEnabled = enabled button.isUserInteractionEnabled = enabled } |
That’s much better, but just imagine, that at some point in time the isUserInteractionEnabled would be renamed to userInteractionEnabled. Searching through the code in order to find the usages would be a pain. Lets try to avoid calling isUserInteractionEnabled multiple times to achieve an even better deduplication:
|
1 2 3 4 |
func betterThanAverageOnSwitch(sender: UISwitch) { [view, button].forEach { $0.isUserInteractionEnabled = sender.isOn } } |
Whoa. That code is short. But is it good? On the visual level, there is no duplication. However, the duplication is still in there. It’s a logical duplication, as we call isOn in for each (twice for that particular case). So, lets cache it, shall we?
|
1 2 3 4 5 |
func nearlyThereOnSwitch(sender: UISwitch) { let enabled = sender.isOn [view, button].forEach { $0.isUserInteractionEnabled = enabled } } |
That’s much better. Now lets follow my advice and think about the future usages. I don’t have anything against YAGNI (you ain’t gonna need it) principle, I only propose to think about the decomposition, as small chainable functions is a much better way of writing, changing and reusing your ideas. Suppose we’d have to add another view, that we’d like to be disabled. Suppose, that we’d like to not only enable or disable the buttons but to change their background colors to the same color. Suppose, that we’d like to be able to enable/disable views in a completely different unrelated piece of code. The solution is simple and reusable at its core:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 |
typealias Views = [UIView] func views() -> Views { return [view, button] } func enable(views: Views, enabled: Bool) { views.forEach { $0.isUserInteractionEnabled = enabled } } func onSwitch(sender: UISwitch) { enable(views: views(), enabled: sender.isOn) } |
Lets analyze it:
- We create the typealias to make the code more readable and avoid writing difficult syntax constructs. Yep, I consider writing UIView in square braces in several places of your code difficult, as it requires additional key strokes and autocomplete wouldn’t be able to help you;
- We create the accessor to the array of views, if we want to change the views to be processed, we do that in just one access point and we avoid possible duplication;
- We create enable, that could be used in different places of your code without the need to rewrite the same method or use the inheritance.
It could be improved even more, if the properties were first order functions (they are under the compiler hood, just check the AST, but Swift won’t let us use them that way). We could, of course, emulate that behavior by passing the closure:
|
1 2 3 4 5 6 7 8 9 |
typealias ViewFunction = (UIView) -> () func firstOrderEnable(enabled: Bool) -> ViewFunction { return { $0.isUserInteractionEnabled = enabled } } func firstOrderOnSwitch(sender: UISwitch) { views().forEach(firstOrderEnable(enabled: sender.isOn)) } |
That’s just the matter of taste and a of a particular task. I don’t always use that approach, but I can’t say I avoid it all the time either. Just remember, for both cases, you’d have to deduplicate forEach in views in case you iterate them in multiple methods like that (a rough example, as, once again, based on the task, you could come up with a better way of solving the problem):
|
1 2 3 4 5 6 7 8 |
func forEachView(_ f: ViewFunction) { views().forEach(f) } func deduplicatedOnSwitch(sender: UISwitch) { forEachView(firstOrderEnable(enabled: sender.isOn)) } |
I’ll discuss higher order functions some time later in more detail, for now, just try the approaches I outlined above, you’ll be shocked at the ways you could apply them in order to simplify and deduplicate your code.
That’s all, folks. Have a great day and be DRY, no matter, where you are.
(No Ratings Yet)
Loading...