#97: First iteration of refactoring #90

isakovch · 2023-10-29T13:45:18Z

Changes:

Replaced ktlint with more popular plugin and updated rules
Updated build properties and parameters
Replaced Dagger with Koin
Splitted Composable functions in EditCanvas
Changed the project structure to feature type not layer
Started cleaning fields through [EditionState]
Marked several classes as deprecated since they are violating SOLID principles
Started integrating Version catalogs
Removed pre commit, since we already have a ktlint check in CI/CD
Added common package for future core classes
Preparation to Java 11 migration

Future plans:

Continue cleaning fields and moving them into [EditionState] class
Splitting EditManager's responsibilities
Toml integration & libs update
Java 11 migration
All Canvas screens cleaning

…ill be fixed later

kirillt · 2023-10-31T08:21:36Z

app/src/main/java/dev/arkbuilders/arkretouch/edition/manager/EditManager.kt

    private val _isCropMode = mutableStateOf(false)
    val isCropMode = _isCropMode

+    var editionMode: EditionMode by mutableStateOf(EditionMode.IDLE)


EditingMode sounds more correct. Could you create an issue for refactoring of operations as EditingModes?

kirillt · 2023-10-31T08:22:13Z

app/src/main/java/dev/arkbuilders/arkretouch/edition/manager/EditManager.kt

 import timber.log.Timber
 import java.util.Stack

+// FIXME: This class is overloaded, split into smaller classes/managers


Same issue as EditingMode or separate?

Extract different modes of operation as separate EditingMode classes #93

kirillt · 2023-10-31T08:22:54Z

app/src/main/java/dev/arkbuilders/arkretouch/edition/ui/main/EditViewModel.kt

    val editManager = EditManager()

-    var strokeSliderExpanded by mutableStateOf(false)
+    // TODO: Move each field into [EditionState] in next PRs


EditingState sounds more correct. Same issue as EditingMode?

No, this is a diferent one

Refactor EditViewModel using EditingState #94

mdrlzy

No changes in logic
Crash when opening edit screen

Crash

2023-10-31 14:18:55.757 15709-15709 [Koin] dev.arkbuilders.arkretouch2 E  * Instance creation error : could not create instance for '[Factory:'dev.arkbuilders.arkretouch.edition.ui.main.EditViewModel']': org.koin.core.error.NoBeanDefFoundException: No definition found for class:'java.lang.String' q:''. Check your definitions!
	org.koin.core.scope.Scope.throwDefinitionNotFound(Scope.kt:304)
	org.koin.core.scope.Scope.resolveValue(Scope.kt:274)
	org.koin.core.scope.Scope.resolveInstance(Scope.kt:231)
	org.koin.core.scope.Scope.get(Scope.kt:210)
	dev.arkbuilders.arkretouch.edition.EditionModule$create$1$invoke$$inlined$viewModelOf$default$1.invoke(ViewModelOf.kt:231)
	dev.arkbuilders.arkretouch.edition.EditionModule$create$1$invoke$$inlined$viewModelOf$default$1.invoke(ViewModelOf.kt:96)
	org.koin.core.instance.InstanceFactory.create(InstanceFactory.kt:51)
	org.koin.core.instance.FactoryInstanceFactory.get(FactoryInstanceFactory.kt:38)
	org.koin.core.registry.InstanceRegistry.resolveInstance$koin_core(InstanceRegistry.kt:116)
	org.koin.core.scope.Scope.resolveValue(Scope.kt:246)
	org.koin.core.scope.Scope.resolveInstance(Scope.kt:231)
	org.koin.core.scope.Scope.get(Scope.kt:210)
	org.koin.androidx.viewmodel.factory.KoinViewModelFactory.create(KoinViewModelFactory.kt:25)
	androidx.lifecycle.ViewModelProvider.get(ViewModelProvider.kt:187)
	androidx.lifecycle.ViewModelProvider.get(ViewModelProvider.kt:153)
	org.koin.androidx.viewmodel.GetViewModelKt.resolveViewModel(GetViewModel.kt:44)
	dev.arkbuilders.arkretouch.edition.ui.main.EditScreenKt.EditScreen(EditScreen.kt:1084)
	dev.arkbuilders.arkretouch.main.MainScreenKt$MainScreen$2$5.invoke(MainScreen.kt:94)
	dev.arkbuilders.arkretouch.main.MainScreenKt$MainScreen$2$5.invoke(MainScreen.kt:93)
	androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:116)
	androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:34)
	androidx.navigation.compose.NavHostKt$NavHost$4$2.invoke(NavHost.kt:163)
	androidx.navigation.compose.NavHostKt$NavHost$4$2.invoke(NavHost.kt:162)
	androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:107)
	androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:34)
	androidx.compose.runtime.CompositionLocalKt.CompositionLocalProvider(CompositionLocal.kt:228)
	androidx.compose.runtime.saveable.SaveableStateHolderImpl.SaveableStateProvider(SaveableStateHolder.kt:84)
	androidx.navigation.compose.NavBackStackEntryProviderKt.SaveableStateProvider(NavBackStackEntryProvider.kt:60)
	androidx.navigation.compose.NavBackStackEntryProviderKt.access$SaveableStateProvider(NavBackStackEntryProvider.kt:1)
	androidx.navigation.compose.NavBackStackEntryProviderKt$LocalOwnersProvider$1.invoke(NavBackStackEntryProvider.kt:52)
	androidx.navigation.compose.NavBackStackEntryProviderKt$LocalOwnersProvider$1.invoke(NavBackStackEntryProvider.kt:51)
	androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:107)
	androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:34)
	androidx.compose.runtime.CompositionLocalKt.CompositionLocalProvider(CompositionLocal.kt:228)
	androidx.navigation.compose.NavBackStackEntryProviderKt.LocalOwnersProvider(NavBackStackEntryProvider.kt:47)
	androidx.navigation.compose.NavHostKt$NavHost$4.invoke(NavHost.kt:162)
	androidx.navigation.compose.NavHostKt$NavHost$4.invoke(NavHost.kt:141)
	androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:116)
	androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:34)
	androidx.compose.animation.CrossfadeKt$Crossfade$5$1.invoke(Crossfade.kt:133)
	androidx.compose.animation.CrossfadeKt$Crossfade$5$1.invoke(Crossfade.kt:128)
	androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:107)
	androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:34)
	androidx.compose.animation.CrossfadeKt.Crossfade(Crossfade.kt:142)
	androidx.compose.animation.CrossfadeKt.Crossfade(Crossfade
2023-10-31 14:18:55.782 15709-15709 ACRA dev.arkbuilders.arkretouch2 E  ACRA caught a InstanceCreationException for dev.arkbuilders.arkretouch2
	org.koin.core.error.InstanceCreationException: Could not create instance for '[Factory:'dev.arkbuilders.arkretouch.edition.ui.main.EditViewModel']'
	at org.koin.core.instance.InstanceFactory.create(InstanceFactory.kt:58)
	at org.koin.core.instance.FactoryInstanceFactory.get(FactoryInstanceFactory.kt:38)
	at org.koin.core.registry.InstanceRegistry.resolveInstance$koin_core(InstanceRegistry.kt:116)
	at org.koin.core.scope.Scope.resolveValue(Scope.kt:246)
	at org.koin.core.scope.Scope.resolveInstance(Scope.kt:231)
	at org.koin.core.scope.Scope.get(Scope.kt:210)
	at org.koin.androidx.viewmodel.factory.KoinViewModelFactory.create(KoinViewModelFactory.kt:25)
	at androidx.lifecycle.ViewModelProvider.get(ViewModelProvider.kt:187)
	at androidx.lifecycle.ViewModelProvider.get(ViewModelProvider.kt:153)
	at org.koin.androidx.viewmodel.GetViewModelKt.resolveViewModel(GetViewModel.kt:44)
	at dev.arkbuilders.arkretouch.edition.ui.main.EditScreenKt.EditScreen(EditScreen.kt:1084)
	at dev.arkbuilders.arkretouch.main.MainScreenKt$MainScreen$2$5.invoke(MainScreen.kt:94)
	at dev.arkbuilders.arkretouch.main.MainScreenKt$MainScreen$2$5.invoke(MainScreen.kt:93)
	at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:116)
	at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:34)
	at androidx.navigation.compose.NavHostKt$NavHost$4$2.invoke(NavHost.kt:163)
	at androidx.navigation.compose.NavHostKt$NavHost$4$2.invoke(NavHost.kt:162)
	at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:107)
	at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:34)
	at androidx.compose.runtime.CompositionLocalKt.CompositionLocalProvider(CompositionLocal.kt:228)
	at androidx.compose.runtime.saveable.SaveableStateHolderImpl.SaveableStateProvider(SaveableStateHolder.kt:84)
	at androidx.navigation.compose.NavBackStackEntryProviderKt.SaveableStateProvider(NavBackStackEntryProvider.kt:60)
	at androidx.navigation.compose.NavBackStackEntryProviderKt.access$SaveableStateProvider(NavBackStackEntryProvider.kt:1)
	at androidx.navigation.compose.NavBackStackEntryProviderKt$LocalOwnersProvider$1.invoke(NavBackStackEntryProvider.kt:52)
	at androidx.navigation.compose.NavBackStackEntryProviderKt$LocalOwnersProvider$1.invoke(NavBackStackEntryProvider.kt:51)
	at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:107)
	at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:34)
	at androidx.compose.runtime.CompositionLocalKt.CompositionLocalProvider(CompositionLocal.kt:228)
	at androidx.navigation.compose.NavBackStackEntryProviderKt.LocalOwnersProvider(NavBackStackEntryProvider.kt:47)
	at androidx.navigation.compose.NavHostKt$NavHost$4.invoke(NavHost.kt:162)
	at androidx.navigation.compose.NavHostKt$NavHost$4.invoke(NavHost.kt:141)
	at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:116)
	at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:34)
	at androidx.compose.animation.CrossfadeKt$Crossfade$5$1.invoke(Crossfade.kt:133)
	at androidx.compose.animation.CrossfadeKt$Crossfade$5$1.invoke(Crossfade.kt:128)
	at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:107)
	at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:34)
	at androidx.compose.animation.CrossfadeKt.Crossfade(Crossfade.kt:142)
	at androidx.compose.animation.CrossfadeKt.Crossfade(Crossfade.kt:73)
	at androidx.navigation.compose.NavHostKt.NavHost(NavHost.kt:141)
	at androidx.navigation.compose.NavHostKt$NavHost$5.invoke(Unknown Source:13)
	at androidx.navigation.compose.NavHostKt$NavHost$5.invoke(Unknown Source:10)
	at androidx.compose.runtime.RecomposeScopeImpl.compose(RecomposeScopeImpl.kt:145)
2023-10-31 14:18:55.783 15709-15709 ACRA dev.arkbuilders.arkretouch2 E  	at androidx.compose.runtime.ComposerImpl.recomposeToGroupEnd(Composer.kt:2375)
	at androidx.compose.runtime.ComposerImpl.skipCurrentGroup(Composer.kt:2643)
	at androidx.compose.runtime.ComposerImpl$doCompose$2$5.invoke(Composer.kt:3260)
	at androidx.compose.runtime.ComposerImpl$doCompose$2$5.invoke(Composer.kt:3238)
	at androidx.compose.runtime.SnapshotStateKt__DerivedStateKt.observeDerivedStateRecalculations(DerivedState.kt:341)
	at androidx.compose.runtime.SnapshotStateKt.observeDerivedStateRecalculations(Unknown Source:1)
	at androidx.compose.runtime.ComposerImpl.doCompose(Composer.kt:3238)
	at androidx.compose.runtime.ComposerImpl.recompose$runtime_release(Composer.kt:3203)
	at androidx.compose.runtime.CompositionImpl.recompose(Composition.kt:771)
	at androidx.compose.runtime.Recomposer.performRecompose(Recomposer.kt:1031)
	at androidx.compose.runtime.Recomposer.access$performRecompose(Recomposer.kt:125)
	at androidx.compose.runtime.Recomposer$runRecomposeAndApplyChanges$2$2.invoke(Recomposer.kt:534)
	at androidx.compose.runtime.Recomposer$runRecomposeAndApplyChanges$2$2.invoke(Recomposer.kt:503)
	at androidx.compose.ui.platform.AndroidUiFrameClock$withFrameNanos$2$callback$1.doFrame(AndroidUiFrameClock.android.kt:34)
	at androidx.compose.ui.platform.AndroidUiDispatcher.performFrameDispatch(AndroidUiDispatcher.android.kt:109)
	at androidx.compose.ui.platform.AndroidUiDispatcher.access$performFrameDispatch(AndroidUiDispatcher.android.kt:41)
	at androidx.compose.ui.platform.AndroidUiDispatcher$dispatchCallback$1.doFrame(AndroidUiDispatcher.android.kt:69)
	at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1029)
	at android.view.Choreographer.doCallbacks(Choreographer.java:854)
	at android.view.Choreographer.doFrame(Choreographer.java:785)
	at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1016)
	at android.os.Handler.handleCallback(Handler.java:883)
	at android.os.Handler.dispatchMessage(Handler.java:100)
	at android.os.Looper.loop(Looper.java:224)
	at android.app.ActivityThread.main(ActivityThread.java:7562)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:539)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:950)
Suppressed: kotlinx.coroutines.DiagnosticCoroutineContextException: [androidx.compose.runtime.PausableMonotonicFrameClock@13285f4, androidx.compose.ui.platform.MotionDurationScaleImpl@60b911d, StandaloneCoroutine{Cancelling}@291cf92, AndroidUiDispatcher@2400d63]
Caused by: org.koin.core.error.NoBeanDefFoundException: No definition found for class:'java.lang.String' q:''. Check your definitions!
	at org.koin.core.scope.Scope.throwDefinitionNotFound(Scope.kt:304)
	at org.koin.core.scope.Scope.resolveValue(Scope.kt:274)
	at org.koin.core.scope.Scope.resolveInstance(Scope.kt:231)
	at org.koin.core.scope.Scope.get(Scope.kt:210)
	at dev.arkbuilders.arkretouch.edition.EditionModule$create$1$invoke$$inlined$viewModelOf$default$1.invoke(ViewModelOf.kt:231)
	at dev.arkbuilders.arkretouch.edition.EditionModule$create$1$invoke$$inlined$viewModelOf$default$1.invoke(ViewModelOf.kt:96)
	at org.koin.core.instance.InstanceFactory.create(InstanceFactory.kt:51)
	... 70 more

kirillt · 2023-10-31T08:26:30Z

app/src/main/java/dev/arkbuilders/arkretouch/App.kt

+        setupAcra()
+    }
+
+    // FIXME: Let's use Firebase crashlytics instead


Integrate full-featured reporting system #91

kirillt · 2023-10-31T08:29:00Z

app/src/main/java/dev/arkbuilders/arkretouch/edition/ui/main/EditViewModel.kt

-        name: String = ""
-    ) = getCachedImageUri(context, bitmap, name)
-
+    // FIXME: Remove context or this function from here


Refactor EditViewModel #92

kirillt · 2023-10-31T08:29:06Z

app/src/main/java/dev/arkbuilders/arkretouch/edition/ui/main/EditViewModel.kt

@@ -139,6 +144,7 @@ class EditViewModel(
            isSavingImage = false
        }

+    // FIXME: Remove context or this function from here


Refactor EditViewModel #92

kirillt · 2023-10-31T08:29:18Z

app/src/main/java/dev/arkbuilders/arkretouch/edition/ui/main/EditViewModel.kt

-    }
-}
-
+// FIXME: Remove context or this function from here


Refactor EditViewModel #92

kirillt · 2023-10-31T08:29:25Z

app/src/main/java/dev/arkbuilders/arkretouch/edition/ui/main/EditViewModel.kt

@@ -454,6 +428,7 @@ private fun loadImageWithPath(
        .loadInto(editManager)
 }

+// FIXME: Remove context or this function from here


Refactor EditViewModel #92

kirillt · 2023-10-31T08:29:33Z

app/src/main/java/dev/arkbuilders/arkretouch/edition/ui/main/EditViewModel.kt

@@ -464,12 +439,14 @@ private fun loadImageWithUri(
        .loadInto(editManager)
 }

+// FIXME: Remove context or this function from here


Refactor EditViewModel #92

kirillt · 2023-10-31T08:29:41Z

app/src/main/java/dev/arkbuilders/arkretouch/edition/ui/main/EditViewModel.kt

 private fun initGlideBuilder(context: Context) = Glide
    .with(context)
    .asBitmap()
    .skipMemoryCache(true)
    .diskCacheStrategy(DiskCacheStrategy.NONE)

+// FIXME: Remove context or this function from here


Refactor EditViewModel #92

kirillt · 2023-10-31T08:30:44Z

app/src/main/java/dev/arkbuilders/arkretouch/storage/ImageDefaults.kt


 import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.unit.IntSize
 import kotlinx.serialization.Serializable

+// FIXME: Replace with Parcelable


Is it worth of creating follow-up issue for this? Or can be done in this PR?

We need issue since it can lead to crashes

Implement Parcelable for ImageDefaults #95

kirillt · 2023-11-01T07:28:25Z

The crash and todos moved to these issues:

First iteration of refactoring

ea844a2

isakovch self-assigned this Oct 29, 2023

- Revert gradle.properties, since CI/CD is not support cache. CI/CD w…

4cb6d6d

…ill be fixed later

isakovch temporarily deployed to Development October 29, 2023 13:49 — with GitHub Actions Inactive

isakovch had a problem deploying to Development October 29, 2023 13:54 — with GitHub Actions Failure

isakovch temporarily deployed to Development October 29, 2023 13:54 — with GitHub Actions Inactive

NO-TICKET - Ktlint fixes

3982a68

isakovch temporarily deployed to Development October 29, 2023 18:33 — with GitHub Actions Inactive

isakovch temporarily deployed to Development October 29, 2023 18:35 — with GitHub Actions Inactive

isakovch requested review from mdrlzy and kirillt October 30, 2023 16:47

kirillt reviewed Oct 31, 2023

View reviewed changes

mdrlzy reviewed Oct 31, 2023

View reviewed changes

kirillt reviewed Oct 31, 2023

View reviewed changes

kirillt mentioned this pull request Nov 1, 2023

org.koin.core.error.NoBeanDefFoundException #96

Closed

kirillt changed the base branch from main to develop November 1, 2023 07:28

kirillt merged commit 0b56a78 into develop Nov 1, 2023
3 checks passed

kirillt mentioned this pull request Nov 1, 2023

Complete refactoring with best practices #97

Open

1 task

kirillt changed the title ~~First iteration of refactoring~~ #97: First iteration of refactoring Nov 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#97: First iteration of refactoring #90

#97: First iteration of refactoring #90

isakovch commented Oct 29, 2023 •

edited

Loading

kirillt Oct 31, 2023

kirillt Oct 31, 2023

kirillt Nov 1, 2023

kirillt Oct 31, 2023

isakovch Oct 31, 2023

kirillt Nov 1, 2023

mdrlzy left a comment •

edited by kirillt

Loading

kirillt Oct 31, 2023

kirillt Oct 31, 2023

kirillt Oct 31, 2023

kirillt Oct 31, 2023

kirillt Oct 31, 2023

kirillt Oct 31, 2023

kirillt Oct 31, 2023

kirillt Oct 31, 2023

isakovch Oct 31, 2023

kirillt Nov 1, 2023

kirillt commented Nov 1, 2023

#97: First iteration of refactoring #90

#97: First iteration of refactoring #90

Conversation

isakovch commented Oct 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdrlzy left a comment • edited by kirillt Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kirillt commented Nov 1, 2023

isakovch commented Oct 29, 2023 •

edited

Loading

mdrlzy left a comment •

edited by kirillt

Loading