amplitude / amplitude/ampli-examples

Make `client` optional to avoid unexpected crashes

未關閉
#328 0 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視
主要語言
Java
星號
12
分支
12
PR 合併指標
30 天內沒有已合併 PR

描述

I would recommend to make `client` optional as this may cause unexpected crashes. You have the following code on the CLI.

```
public class Ampli {
private var amplitude: Amplitude?
public var client: Amplitude {
get {
_ = isInitializedAndEnabled()
return amplitude!
}
}

private func isInitializedAndEnabled() -> Bool {
if !self.isLoaded {
NSLog("Ampli is not yet initialized. Have you called `ampli.load()` on app start?")
return false
}
return !self.disabled
}
```
I have some questions about this code:
1 - If `var amplitude` is optional, why are you force unwrapping the client? This may cause (and caused us) some issues when for example calling `Ampli.instance.client.reset()` as of `amplitude` is nil, the application will crash.
**Probably you'll say that I should call `load()` before, but in our case, we call the load method after an async completion callback, so it still can happen.
2 - What `_ = isInitializedAndEnabled()` does? Only returning an ignored true/false value.
3 - Why creating an optional and private `amplitude` property and then giving access with a public computed variable that returns the other private one? Wouldn't be the same to do:
`public fileprivate(set) var amplitude: Amplitude?`

貢獻指南

這個儲存庫沒有索引到貢獻指南

評估

這個 Issue 還沒有評估資料。

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。