First of all: 馃檱 Thanks for all the hard work you put into this project in the last couple of weeks!
Second: Nitpicking income - but I'd love to take care of that if you think it's worth it.
You use NSLayoutConstraints at various places in the code. Even for the views where you make extensive usage of them, you mostly stick to use the isActive = true API to enable each constraint individually - even for classes like DynamicTableViewRoundedCell (where you enable 11 constraints at one place).
According to Apple's UIKit documentation using the type method NSLayoutConstraint.activate(_:) is more efficient when enabling multiple constraints:
[...]
Typically, using this method is more efficient than activating each constraint individually.
[...]
See here: Documentation
Currently, most constraints are activated individually by using the isActive instance property.
In code pieces where multiple constraints are activated at the same time, it would be worth to activate all of them in one NSLayoutConstraint.activate(_:) call.
(As it's for example done in the OnboardingInfoViewController)
(Minor) efficiency improvements.
@inf2381 before you get to it I would suggest a slightly different approach which should be compatible with what @lennartstolz has in mind:
It is true that +NSLayoutConstraint.activate(constraints:) is more performant than activating constraints individually. +NSLayoutConstraint.activate(constraints:) has one big drawback though:
If any constraint you are trying to activate is causing an auto layout warning/error/exception you don't know immediately which one it is. The debugger halts at +NSLayoutConstraint.activate(constraints:) and not at the constraint that caused the issue.
This could be improved by setting identifier on the constraint or by activating those constraints individually.
To get the best of both worlds - what do you think about:
extension Array where Element == NSLayoutConstraint {
func activate() {
#if DEBUG
for constraint in self {
constraint.isActive = true
}
#else
NSLayoutConstraint.activate(self)
#endif
}
}
// Can be used like this:
[
label.leadingAnchor.constraint(equalTo: leadingAnchor),
label.topAnchor.constraint(equalTo: topAnchor)
].activate()
Thanks, @ChristianKienle for sharing your concerns which make a lot of sense.
Not sure if your suggested solution really would help to keep the ease of debugging in the same way as you currently do it. The debugger would halt inside the for loop (because there it breaks) and you'd need to use the LLDB debugger or similar to get to know which constraint it is, or do I miss something?
Thanks, @ChristianKienle for sharing your concerns which make a lot of sense.
Not sure if your suggested solution really would help to keep the ease of debugging in the same way as you currently do it. The debugger would halt inside the
forloop (because there it breaks) and you'd need to use the LLDB debugger or similar to get to know which constraint it is, or do I miss something?
Yeah but at least you know the constraint that caused the error.
po firstIndex(of: constraint)
And then you know which constraint caused the issue. This information is lot when the constraints are no longer activated individually.
I completely agree that the suggested solution would help to combine the best of both worlds.
You still have a way to debug and also you get the performance improvements by batch activating them on !DEBUG builds. Sad that there is no way to keep the ease of debugging when doing those improvements.
Any updates here?
Edit:
I think they use NSLayoutConstraint.activate(_:) for activating constrains now.
Maybe @dsarkar could ask the devs if I think correct (if yes: this issue would be solved)
@Ein-Tim Will check if they need this open.
If you decide it worth tackling and fits the overall project strategy, I'd love to tackle it.
Most helpful comment
@inf2381 before you get to it I would suggest a slightly different approach which should be compatible with what @lennartstolz has in mind:
It is true that
+NSLayoutConstraint.activate(constraints:)is more performant than activating constraints individually.+NSLayoutConstraint.activate(constraints:)has one big drawback though:If any constraint you are trying to activate is causing an auto layout warning/error/exception you don't know immediately which one it is. The debugger halts at
+NSLayoutConstraint.activate(constraints:)and not at the constraint that caused the issue.This could be improved by setting
identifieron the constraint or by activating those constraints individually.To get the best of both worlds - what do you think about: