garretyoder / Colorful

Android runtime theme library
Apache License 2.0
2.13k stars 194 forks source link

Colorful has error when start strict mode #36

Closed tcqq closed 6 years ago

tcqq commented 6 years ago

Colorful has error when start strict mode

image

    override fun onCreate() {
        if (BuildConfig.DEBUG) {
            setStrictMode()
        }
        super.onCreate()
    }

    private fun setStrictMode() {
        StrictMode.setThreadPolicy(StrictMode.ThreadPolicy.Builder()
                .detectDiskReads()
                .detectDiskWrites()
                .detectNetwork()   // or .detectAll() for all detectable problems
                .penaltyLog()
                .build())
        StrictMode.setVmPolicy(StrictMode.VmPolicy.Builder()
                .detectLeakedSqlLiteObjects()
                .detectLeakedClosableObjects()
                .penaltyLog()
                .penaltyDeath()
                .build())
    }
tcqq commented 6 years ago

I fixed the above error log in this way.

abstract class BaseApp : Application(), Application.ActivityLifecycleCallbacks {

    override fun onCreate() {
        if (BuildConfig.DEBUG) {
            setStrictMode()
        }
        super.onCreate()

        registerActivityLifecycleCallbacks(this)

            Thread {
                val defaults = Defaults(
                        primaryColor(),
                        accentColor(),
                        isDark(),
                        isTranslucent(),
                        customTheme())
                initColorful(this, defaults)
            }.start()

    override fun onActivityCreated(activity: Activity, bundle: Bundle?) {
            AppCompatDelegate.setDefaultNightMode(if (Colorful().getDarkTheme())
                AppCompatDelegate.MODE_NIGHT_YES
            else
                AppCompatDelegate.MODE_NIGHT_NO)
        }
}
garretyoder commented 6 years ago

Why are you using StrictMode?

tcqq commented 6 years ago

Best practice in Android says “keeping the disk and network operations off from the main thread makes applications much smoother and more responsive”. StrictMode which detects things you might be doing by accident and brings them to your attention so you can fix them.

garretyoder commented 6 years ago

StrictMode is infamous for miss flagging things. If you look at the error, the specific line of Colorful's code it's flagging is the following.

var primary: ThemeColorInterface = ThemeColorInterface.parse(prefs.getString(primaryThemeKey, defaults.primaryColor.themeName))

This is just a simple shared preference read. SharedPreferences is already correctly managed by the android system. This is not a bug in colorful and a nonissue, closing.

tcqq commented 6 years ago

Colorful code uses SharedPreferences disk read operation, running all I/O as non-blocking off the main thread is ideal.

garretyoder commented 6 years ago

You're welcome to submit a PR to change the data read source to somewhere else, however as I said, strict mode isn't really a good thing to use in production. As soon as you get different devices that perform operations at different speeds, you end up triggering strict mode on some devices but others are fine. Strict mode is meant to encourage good practices. Did you see the time the message gave you? 25ms. That's a very insignificant amount of time for a load, hell it takes Android longer to create a Theme object, but that's ignored by strict mode.

If it irks you that much submit a PR, but I'm considering this a non-issue because this won't affect performance in the slightest or block a thread for any meaningful amount of time.

Hell, calling super.onCreate in a activity is a longer running operation than that.

tcqq commented 6 years ago

Thanks for reply, I understand.