wnagrodzki / iOSProgrammingGuidelines

2 stars 0 forks source link

Assertions, preconditions and fatal errors #2

Closed wnagrodzki closed 5 years ago

wnagrodzki commented 6 years ago

Use precondition(_:_:file:line:) for internal sanity checks by default. If you do not want to impact performance of shipping code apply assert(_:_:file:line:) instead.

This is to avoid internal sanity checks being removed in optimized builds. Using preconditions to enforce valid data and state causes your app to terminate more predictably if an invalid state occurs, and helps make the problem easier to debug. Stopping execution as soon as an invalid state is detected also helps limit the damage caused by that invalid state.

Utilize fatalError(_:file:line:) in places where application's state and logic are not evaluated at all, for example: unimplemented method stubs.


More information can be found in Swift Asserts article. Related: Proper use of asserts "Avoid force unwrapping" section should be changed so fatalError is not used there

pwetrifork commented 6 years ago

I was wondering if there is a way to control console output in release builds. The documentation for precondition() mentions the following:

In playgrounds and -Onone builds (the default for Xcode’s Debug configuration): If condition evaluates to false, stop program execution in a debuggable state after printing message. In -O builds (the default for Xcode’s Release configuration): If condition evaluates to false, stop program execution.

This sounds like precondition failure will not print anything in release builds.

wnagrodzki commented 6 years ago

@pwetrifork What are you planning to gain here? Crash report will point to exact line that crashed and logs will be gone quite fast as iOS seems to be logging heavily nowadays.

wnagrodzki commented 5 years ago

@moskacz @pwetrifork What do you think (text below horizontal line is not to be included in the guidelines)?

pwetrifork commented 5 years ago

We could also add a guideline to use preconditionFailure with guard for sanity checks involving optional unwrapping

wnagrodzki commented 5 years ago

@pwetrifork Look at docs regarding -Ounchecked builds:

In -Ounchecked builds, the optimizer may assume that this function is never called. Failure to satisfy that assumption is a serious programming error.

Do you know what we can expect in that situation?

pwetrifork commented 5 years ago

I would assume that going with the unchecked optimization is an extreme scenario where you’re 110% sure the app is bug free and you don’t care about undefined behavior. I don’t remember us ever using that option.

wnagrodzki commented 5 years ago

@pwetrifork We did not use it before. What is the benefit of using preconditionFailure over fatalError?

pwetrifork commented 5 years ago

I would use that for consistency between plain precondition checks and the ones that involve “required optionals”. Another difference between fatalError and preconditionFailure is that the latter does not print anything to the console in release builds, so it’s less likely to leak sensitive information.

wnagrodzki commented 5 years ago

Very good points, I agree. That leaves us with using fatalErrors for “dead ends”. Would you agree?

pwetrifork commented 5 years ago

Yes I would leave that for unimplemented method stubs etc., never interleaved with any actual application logic

Moskacz commented 5 years ago

Everything is clear for me