kstenerud / KSCrash

The Ultimate iOS Crash Reporter
MIT License
4.23k stars 705 forks source link

Use protocol for crash report type #527

Closed bamx23 closed 1 month ago

bamx23 commented 2 months ago

The 3-values crash report type wasn't a good idea form an API side of view as some users were confused that stringValue would automatically convert crash report to string.

This PR replaces KSCrashReport class with an abstract protocol. This also allows writing custom filters with user-defined representations of crash report (e.g. protobuf or something else).

GLinnik21 commented 2 months ago

Addresses https://github.com/kstenerud/KSCrash/issues/525#issuecomment-2210816546

GLinnik21 commented 2 months ago

Ok, now we have to use isKindOfClass: everywhere, but I believe it's a fair tradeoff.

The bigger question is how the value looks. In Swift, it will be Any. This means that the user now has to know exactly which class to cast the value to. Previously, the user had three options. Alternatively, the user can cast the CrashReport protocol to a concrete implementation.

I'm not sure which is more confusing, the new implementation with Any or the current one.

bamx23 commented 2 months ago

I've added some extra text to the protocol docs. In the protocol itself there's a untypedValue that anyone can use and unsafely cast to whatever they want. And for type safety they can cast to a specific implementation first (and a value there is now guaranteed to be non-nil). I've added an example of a switch to the sample.

Also, as only filters can change the type of the report, the KSCrash class now returns dictionary variant directly making it more straight-forward to use that API.