Esri / data-collection-ios

Mobile data collection app using the iOS Runtime SDK.
https://developers.arcgis.com/
Apache License 2.0
25 stars 26 forks source link

Drops conformance to AppErrors and reworks errors as members of related types #263

Closed esreli closed 3 years ago

esreli commented 3 years ago

This PR is attempting to:

Note, i'm not sure how the removal of LastSyncMobileMapPackage.swift from the build phase occurred, my best guess is that it is a legacy token left behind by a previous improper Xcode removal and that Xcode is attempting to self-correct.

esreli commented 3 years ago

@philium At first glance I thought the same you did in this suggestion. Believe it or not, adherence to LocalizedError requires errorDescription to be declared as optional. Without doing so, a generic localized description is derived, ignoring the computed localized description i've submit in this PR. Here's how specifying the computed value as String vs String? fares when shown in an alert:

var errorDescription: String? var errorDescription: String
Simulator Screen Shot - iPhone 11 Pro - 2020-11-24 at 16 54 08 Simulator Screen Shot - iPhone 11 Pro - 2020-11-24 at 16 55 03

I'll leave as is and will keep this requirement bookmarked- because i'll need to update other errors that conform to LocalizedError.

philium commented 3 years ago

That is fascinating! That must be due to the default implementations provided. Normally it wouldn't be a problem to supply a non-optional property to satisfy the requirement for an optional one.

esreli commented 3 years ago

https://github.com/Esri/data-collection-ios/pull/263#issuecomment-733399131 It is! I had operated under the same assumption and it took me some time to debug because it didn't cross my mind that the optional was strictly required. Though it is nice a default implementation is provided.

esreli commented 3 years ago

@mhdostal @philium The old AppError design pattern is now removed from the app. This PR is ready for review. @philium thank you for all of your help thus far!