From 16b8bd35a3d97f03e3efcd2e5197cfdf720134bc Mon Sep 17 00:00:00 2001 From: Sad Ellie Date: Sat, 24 Dec 2022 13:45:00 +0400 Subject: [PATCH] Fix (?) for input conversion (#12) - Now doing conversion in coroutine. - Little refactor for MainViewModel - Text fields are limited to 1000 symbols now - Little bug added. Conversion result can get really slow when converting extremely large numbers 99^999999 (like really big). Very rare case and user would probably never enter such values. Can't find a proper fix yet. --- .../unitto/data/units/AllUnitsRepository.kt | 7 +- .../com/sadellie/unitto/data/units/MyUnit.kt | 3 +- .../java/com/sadellie/unitto/screens/Utils.kt | 33 +++ .../unitto/screens/main/MainViewModel.kt | 219 ++++++++++-------- .../screens/main/components/MyTextField.kt | 6 +- .../unitto/screens/MainViewModelTest.kt | 6 +- 6 files changed, 167 insertions(+), 107 deletions(-) diff --git a/app/src/main/java/com/sadellie/unitto/data/units/AllUnitsRepository.kt b/app/src/main/java/com/sadellie/unitto/data/units/AllUnitsRepository.kt index 1bfef209..007d1e1a 100644 --- a/app/src/main/java/com/sadellie/unitto/data/units/AllUnitsRepository.kt +++ b/app/src/main/java/com/sadellie/unitto/data/units/AllUnitsRepository.kt @@ -24,6 +24,7 @@ import com.sadellie.unitto.data.preferences.MAX_PRECISION import com.sadellie.unitto.data.units.database.MyBasedUnit import com.sadellie.unitto.screens.setMinimumRequiredScale import com.sadellie.unitto.screens.sortByLev +import com.sadellie.unitto.screens.trimZeros import java.math.BigDecimal import java.math.RoundingMode import javax.inject.Inject @@ -499,7 +500,7 @@ class AllUnitsRepository @Inject constructor() { else -> value } .setMinimumRequiredScale(scale) - .stripTrailingZeros() + .trimZeros() } }, object : AbstractUnit( @@ -529,7 +530,7 @@ class AllUnitsRepository @Inject constructor() { else -> value } .setMinimumRequiredScale(scale) - .stripTrailingZeros() + .trimZeros() } }, object : AbstractUnit( @@ -556,7 +557,7 @@ class AllUnitsRepository @Inject constructor() { else -> value } .setMinimumRequiredScale(scale) - .stripTrailingZeros() + .trimZeros() } }, ) diff --git a/app/src/main/java/com/sadellie/unitto/data/units/MyUnit.kt b/app/src/main/java/com/sadellie/unitto/data/units/MyUnit.kt index 05de8fb8..ce6c2696 100644 --- a/app/src/main/java/com/sadellie/unitto/data/units/MyUnit.kt +++ b/app/src/main/java/com/sadellie/unitto/data/units/MyUnit.kt @@ -21,6 +21,7 @@ package com.sadellie.unitto.data.units import androidx.annotation.StringRes import com.sadellie.unitto.data.preferences.MAX_PRECISION import com.sadellie.unitto.screens.setMinimumRequiredScale +import com.sadellie.unitto.screens.trimZeros import java.math.BigDecimal /** @@ -54,6 +55,6 @@ class MyUnit( .multiply(value) .div(unitTo.basicUnit) .setMinimumRequiredScale(scale) - .stripTrailingZeros() + .trimZeros() } } diff --git a/app/src/main/java/com/sadellie/unitto/screens/Utils.kt b/app/src/main/java/com/sadellie/unitto/screens/Utils.kt index fa9e6433..d9583431 100644 --- a/app/src/main/java/com/sadellie/unitto/screens/Utils.kt +++ b/app/src/main/java/com/sadellie/unitto/screens/Utils.kt @@ -30,6 +30,8 @@ import com.sadellie.unitto.data.OPERATORS import com.sadellie.unitto.data.preferences.OutputFormat import com.sadellie.unitto.data.preferences.Separator import com.sadellie.unitto.data.units.AbstractUnit +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.combine import java.math.BigDecimal import java.math.RoundingMode import kotlin.math.floor @@ -273,3 +275,34 @@ fun Sequence.sortByLev(stringA: String): Sequence { .map { it.first } .asSequence() } + +@Suppress("UNCHECKED_CAST") +fun combine( + flow: Flow, + flow2: Flow, + flow3: Flow, + flow4: Flow, + flow5: Flow, + flow6: Flow, + transform: suspend (T1, T2, T3, T4, T5, T6,) -> R +): Flow = combine(flow, flow2, flow3, flow4, flow5, flow6,) { args: Array<*> -> + transform( + args[0] as T1, + args[1] as T2, + args[2] as T3, + args[3] as T4, + args[4] as T5, + args[5] as T6, + ) +} + +/** + * Removes all trailing zeroes. + * + * @throws NumberFormatException if value is bigger than [Double.MAX_VALUE] to avoid memory overflow. + */ +fun BigDecimal.trimZeros(): BigDecimal { + if (this.abs() > BigDecimal.valueOf(Double.MAX_VALUE)) throw NumberFormatException() + + return if (this.compareTo(BigDecimal.ZERO) == 0) BigDecimal.ZERO else this.stripTrailingZeros() +} diff --git a/app/src/main/java/com/sadellie/unitto/screens/main/MainViewModel.kt b/app/src/main/java/com/sadellie/unitto/screens/main/MainViewModel.kt index 58ffd7e8..097614d8 100644 --- a/app/src/main/java/com/sadellie/unitto/screens/main/MainViewModel.kt +++ b/app/src/main/java/com/sadellie/unitto/screens/main/MainViewModel.kt @@ -59,15 +59,22 @@ import com.sadellie.unitto.data.units.database.MyBasedUnit import com.sadellie.unitto.data.units.database.MyBasedUnitsRepository import com.sadellie.unitto.data.units.remote.CurrencyApi import com.sadellie.unitto.data.units.remote.CurrencyUnitResponse +import com.sadellie.unitto.screens.combine import com.sadellie.unitto.screens.toStringWith +import com.sadellie.unitto.screens.trimZeros import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted -import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update +import kotlinx.coroutines.isActive import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import java.math.BigDecimal import java.math.RoundingMode import javax.inject.Inject @@ -80,29 +87,38 @@ class MainViewModel @Inject constructor( private val allUnitsRepository: AllUnitsRepository ) : ViewModel() { - val inputValue: MutableStateFlow = MutableStateFlow(KEY_0) - private val latestInputStack: MutableList = mutableListOf(KEY_0) - private val _inputDisplayValue: MutableStateFlow = MutableStateFlow(KEY_0) + val input: MutableStateFlow = MutableStateFlow(KEY_0) + private val _calculated: MutableStateFlow = MutableStateFlow(null) + private val _result: MutableStateFlow = MutableStateFlow(KEY_0) + private val _latestInputStack: MutableList = mutableListOf(KEY_0) + private val _inputDisplay: MutableStateFlow = MutableStateFlow(KEY_0) private val _isLoadingDatabase: MutableStateFlow = MutableStateFlow(true) private val _isLoadingNetwork: MutableStateFlow = MutableStateFlow(false) private val _showError: MutableStateFlow = MutableStateFlow(false) - private val _calculatedValue: MutableStateFlow = MutableStateFlow(null) private val _userPrefs = userPrefsRepository.userPreferencesFlow.stateIn( - viewModelScope, - SharingStarted.WhileSubscribed(5000), - UserPreferences() + viewModelScope, SharingStarted.WhileSubscribed(5000), UserPreferences() ) - val mainFlow = combine( - _inputDisplayValue, _isLoadingDatabase, _isLoadingNetwork, _showError, _userPrefs - ) { inputDisplayValue, isLoadingDatabase, isLoadingNetwork, showError, _ -> + val mainFlow: StateFlow = combine( + _inputDisplay, + _calculated, + _result, + _isLoadingNetwork, + _isLoadingDatabase, + _showError, + ) { inputValue: String, + calculatedValue: String?, + resultValue: String, + showLoadingNetwork: Boolean, + showLoadingDatabase: Boolean, + showError: Boolean -> return@combine MainScreenUIState( - inputValue = inputDisplayValue, - resultValue = convertValue(), - isLoadingDatabase = isLoadingDatabase, - isLoadingNetwork = isLoadingNetwork, + inputValue = inputValue, + calculatedValue = calculatedValue, + resultValue = resultValue, + isLoadingNetwork = showLoadingNetwork, + isLoadingDatabase = showLoadingDatabase, showError = showError, - calculatedValue = _calculatedValue.value ) }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), MainScreenUIState()) @@ -121,66 +137,62 @@ class MainViewModel @Inject constructor( /** * This function takes local variables, converts values and then causes the UI to update */ - private fun convertValue(): String { + private suspend fun convertInput() { + withContext(Dispatchers.Default) { + while (isActive) { + // First we clean the input from garbage at the end + var cleanInput = input.value.dropLastWhile { !it.isDigit() } - var cleanInput = inputValue.value.dropLastWhile { !it.isDigit() } + // Now we close open brackets that user didn't close + // AUTOCLOSE ALL BRACKETS + val leftBrackets = input.value.count { it.toString() == KEY_LEFT_BRACKET } + val rightBrackets = input.value.count { it.toString() == KEY_RIGHT_BRACKET } + val neededBrackets = leftBrackets - rightBrackets + if (neededBrackets > 0) cleanInput += KEY_RIGHT_BRACKET.repeat(neededBrackets) - // AUTOCLOSE ALL BRACKETS - val leftBrackets = inputValue.value.count { it.toString() == KEY_LEFT_BRACKET } - val rightBrackets = inputValue.value.count { it.toString() == KEY_RIGHT_BRACKET } + // Now we evaluate expression in input + val evaluationResult: BigDecimal = try { + Expressions().eval(cleanInput) + .setScale(_userPrefs.value.digitsPrecision, RoundingMode.HALF_EVEN) + .trimZeros() + } catch (e: Exception) { + when (e) { + is ExpressionException, + is ArrayIndexOutOfBoundsException, + is IndexOutOfBoundsException, + is NumberFormatException, + is ArithmeticException -> { + // Invalid expression, can't do anything further + cancel() + return@withContext + } + else -> throw e + } + } - val neededBrackets = leftBrackets - rightBrackets - if (neededBrackets > 0) { - cleanInput += KEY_RIGHT_BRACKET.repeat(neededBrackets) - } + // Evaluated. Hide calculated result if no expression entered. + // 123.456 will be true + // -123.456 will be true + // -123.456-123 will be false (first minus gets removed, ending with 123.456) + if (input.value.removePrefix(KEY_MINUS).all { it.toString() !in OPERATORS }) { + // No operators + _calculated.update { null } + } else { + _calculated.update { evaluationResult.toStringWith(_userPrefs.value.outputFormat) } + } - // Kotlin doesn't have a multi catch - val calculatedInput = try { - val evaluated = Expressions() - .eval(cleanInput) - .setScale(_userPrefs.value.digitsPrecision, RoundingMode.HALF_EVEN) - .stripTrailingZeros() - /** - * Too long values crash the UI. If the number is too big, we don't do conversion. - * User probably wouldn't need numbers beyond infinity. - */ - if (evaluated.abs() > BigDecimal.valueOf(Double.MAX_VALUE)) { - _calculatedValue.value = null - return "" - } - if (evaluated.compareTo(BigDecimal.ZERO) == 0) BigDecimal.ZERO else evaluated - } catch (e: Exception) { - // Kotlin doesn't have a multi catch - when (e) { - is ExpressionException, is ArrayIndexOutOfBoundsException, is IndexOutOfBoundsException, is NumberFormatException, is ArithmeticException -> return mainFlow.value.resultValue - else -> throw e + // Now we just convert. + // We can use evaluation result here, input is valid + val conversionResult: BigDecimal = unitFrom.convert( + unitTo, evaluationResult, _userPrefs.value.digitsPrecision + ) + + // Converted + _result.update { conversionResult.toStringWith(_userPrefs.value.outputFormat) } + + cancel() } } - - // Ugly way of determining when to hide calculated value - _calculatedValue.value = if (!latestInputStack.none { it in OPERATORS }) { - calculatedInput.toStringWith(_userPrefs.value.outputFormat) - } else { - null - } - - // Converting value using a specified precision - val convertedValue: BigDecimal = unitFrom.convert( - unitTo, calculatedInput, _userPrefs.value.digitsPrecision - ) - - /** - * There is a very interesting bug when trailing zeros are not stripped when input - * consists of ZEROS only (0.00000 as an example). This check is a workaround. If the result - * is zero, than we make sure there are no trailing zeros. - */ - val resultValue = if (convertedValue.compareTo(BigDecimal.ZERO) == 0) { - KEY_0 - } else { - convertedValue.toStringWith(_userPrefs.value.outputFormat) - } - - return resultValue } /** @@ -194,7 +206,7 @@ class MainViewModel @Inject constructor( // Now we change to positive if the group we switched to supports negate if (!clickedUnit.group.canNegate) { - inputValue.update { inputValue.value.removePrefix(KEY_MINUS) } + input.update { input.value.removePrefix(KEY_MINUS) } } // Now setting up right unit (pair for the left one) @@ -305,7 +317,7 @@ class MainViewModel @Inject constructor( * @param[symbolToAdd] Digit/Symbol we want to add, can be any digit 0..9 or a dot symbol */ fun processInput(symbolToAdd: String) { - val lastTwoSymbols = latestInputStack.takeLast(2) + val lastTwoSymbols = _latestInputStack.takeLast(2) val lastSymbol: String = lastTwoSymbols.getOrNull(1) ?: lastTwoSymbols[0] val lastSecondSymbol: String? = lastTwoSymbols.getOrNull(0) @@ -313,8 +325,8 @@ class MainViewModel @Inject constructor( KEY_PLUS, KEY_DIVIDE, KEY_MULTIPLY, KEY_EXPONENT -> { when { // Don't need expressions that start with zero - (inputValue.value == KEY_0) -> {} - (inputValue.value == KEY_MINUS) -> {} + (input.value == KEY_0) -> {} + (input.value == KEY_MINUS) -> {} (lastSymbol == KEY_LEFT_BRACKET) -> {} (lastSymbol == KEY_SQRT) -> {} /** @@ -338,7 +350,7 @@ class MainViewModel @Inject constructor( KEY_0 -> { when { // Don't add zero if the input is already a zero - (inputValue.value == KEY_0) -> {} + (input.value == KEY_0) -> {} (lastSymbol == KEY_RIGHT_BRACKET) -> { processInput(KEY_MULTIPLY) setInputSymbols(symbolToAdd) @@ -353,7 +365,7 @@ class MainViewModel @Inject constructor( KEY_1, KEY_2, KEY_3, KEY_4, KEY_5, KEY_6, KEY_7, KEY_8, KEY_9 -> { // Replace single zero (default input) if it's here when { - (inputValue.value == KEY_0) -> { + (input.value == KEY_0) -> { setInputSymbols(symbolToAdd, false) } (lastSymbol == KEY_RIGHT_BRACKET) -> { @@ -368,7 +380,7 @@ class MainViewModel @Inject constructor( KEY_MINUS -> { when { // Replace single zero with minus (to support negative numbers) - (inputValue.value == KEY_0) -> { + (input.value == KEY_0) -> { setInputSymbols(symbolToAdd, false) } // Don't allow multiple minuses near each other @@ -391,7 +403,7 @@ class MainViewModel @Inject constructor( KEY_LEFT_BRACKET -> { when { // Replace single zero with minus (to support negative numbers) - (inputValue.value == KEY_0) -> { + (input.value == KEY_0) -> { setInputSymbols(symbolToAdd, false) } (lastSymbol == KEY_RIGHT_BRACKET) || (lastSymbol in DIGITS) || (lastSymbol == KEY_DOT) -> { @@ -406,10 +418,11 @@ class MainViewModel @Inject constructor( KEY_RIGHT_BRACKET -> { when { // Replace single zero with minus (to support negative numbers) - (inputValue.value == KEY_0) -> {} + (input.value == KEY_0) -> {} (lastSymbol == KEY_LEFT_BRACKET) -> {} - (latestInputStack.filter { it == KEY_LEFT_BRACKET }.size == - latestInputStack.filter { it == KEY_RIGHT_BRACKET }.size) -> {} + (_latestInputStack.filter { it == KEY_LEFT_BRACKET }.size == + _latestInputStack.filter { it == KEY_RIGHT_BRACKET }.size) -> { + } else -> { setInputSymbols(symbolToAdd) } @@ -418,7 +431,7 @@ class MainViewModel @Inject constructor( KEY_SQRT -> { when { // Replace single zero with minus (to support negative numbers) - (inputValue.value == KEY_0) -> { + (input.value == KEY_0) -> { setInputSymbols(symbolToAdd, false) } (lastSymbol == KEY_RIGHT_BRACKET) || (lastSymbol in DIGITS) || (lastSymbol == KEY_DOT) -> { @@ -433,7 +446,7 @@ class MainViewModel @Inject constructor( else -> { when { // Replace single zero with minus (to support negative numbers) - (inputValue.value == KEY_0) -> { + (input.value == KEY_0) -> { setInputSymbols(symbolToAdd, false) } else -> { @@ -442,7 +455,6 @@ class MainViewModel @Inject constructor( } } } - } /** @@ -450,24 +462,24 @@ class MainViewModel @Inject constructor( */ fun deleteDigit() { // Default input, don't delete - if (inputValue.value == KEY_0) return + if (input.value == KEY_0) return - val lastSymbol = latestInputStack.removeLast() + val lastSymbol = _latestInputStack.removeLast() // We will need to delete last symbol from both values val displayRepresentation: String = INTERNAL_DISPLAY[lastSymbol] ?: lastSymbol // If this value are same, it means that after deleting there will be no symbols left, set to default - if (lastSymbol == inputValue.value) { + if (lastSymbol == input.value) { setInputSymbols(KEY_0, false) } else { - inputValue.update { it.removeSuffix(lastSymbol) } - _inputDisplayValue.update { it.removeSuffix(displayRepresentation) } + input.update { it.removeSuffix(lastSymbol) } + _inputDisplay.update { it.removeSuffix(displayRepresentation) } } } /** - * Adds given [symbol] to [inputValue] and [_inputDisplayValue] and updates [latestInputStack]. + * Adds given [symbol] to [input] and [_inputDisplay] and updates [_latestInputStack]. * * By default add symbol, but if [add] is False, will replace current input (when replacing * default [KEY_0] input). @@ -477,15 +489,15 @@ class MainViewModel @Inject constructor( when { add -> { - inputValue.update { it + symbol } - _inputDisplayValue.update { it + displaySymbol } - latestInputStack.add(symbol) + input.update { it + symbol } + _inputDisplay.update { it + displaySymbol } + _latestInputStack.add(symbol) } else -> { - latestInputStack.clear() - inputValue.update { symbol } - _inputDisplayValue.update { displaySymbol } - latestInputStack.add(symbol) + _latestInputStack.clear() + input.update { symbol } + _inputDisplay.update { displaySymbol } + _latestInputStack.add(symbol) } } } @@ -511,7 +523,7 @@ class MainViewModel @Inject constructor( /** * Returns True if can be placed. */ - private fun canEnterDot(): Boolean = !inputValue.value.takeLastWhile { + private fun canEnterDot(): Boolean = !input.value.takeLastWhile { it.toString() !in OPERATORS.minus(KEY_DOT) }.contains(KEY_DOT) @@ -522,6 +534,15 @@ class MainViewModel @Inject constructor( userPrefsRepository.updateLatestPairOfUnits(unitFrom, unitTo) } + private fun startObserving() { + viewModelScope.launch(Dispatchers.Default) { + _userPrefs.collectLatest { convertInput() } + } + viewModelScope.launch(Dispatchers.Default) { + input.collectLatest { convertInput() } + } + } + init { viewModelScope.launch { val initialUserPrefs = userPrefsRepository.userPreferencesFlow.first() @@ -546,6 +567,8 @@ class MainViewModel @Inject constructor( // User is free to convert values and units on units screen can be sorted properly _isLoadingDatabase.update { false } updateCurrenciesBasicUnits() + + startObserving() } } } diff --git a/app/src/main/java/com/sadellie/unitto/screens/main/components/MyTextField.kt b/app/src/main/java/com/sadellie/unitto/screens/main/components/MyTextField.kt index 1de3a579..a4609b3a 100644 --- a/app/src/main/java/com/sadellie/unitto/screens/main/components/MyTextField.kt +++ b/app/src/main/java/com/sadellie/unitto/screens/main/components/MyTextField.kt @@ -117,7 +117,8 @@ fun MyTextField( ) { Text( modifier = Modifier.fillMaxWidth(), - text = it, + // Quick fix to prevent the UI from crashing + text = it.take(1000), textAlign = TextAlign.End, softWrap = false, style = NumbersTextStyleDisplayLarge @@ -151,7 +152,8 @@ fun MyTextField( ) { Text( modifier = Modifier, - text = Formatter.format(it ?: ""), + // Quick fix to prevent the UI from crashing + text = Formatter.format(it?.take(1000) ?: ""), textAlign = TextAlign.End, softWrap = false, color = MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.6f), diff --git a/app/src/test/java/com/sadellie/unitto/screens/MainViewModelTest.kt b/app/src/test/java/com/sadellie/unitto/screens/MainViewModelTest.kt index 158c2485..41aff90d 100644 --- a/app/src/test/java/com/sadellie/unitto/screens/MainViewModelTest.kt +++ b/app/src/test/java/com/sadellie/unitto/screens/MainViewModelTest.kt @@ -177,7 +177,7 @@ class MainViewModelTest { viewModel.processInput(it) viewModel.deleteDigit() assertEquals("0", viewModel.mainFlow.value.inputValue) - assertEquals("0", viewModel.inputValue.value) + assertEquals("0", viewModel.input.value) } viewModel.clearInput() @@ -189,7 +189,7 @@ class MainViewModelTest { viewModel.processInput(KEY_SQRT) viewModel.processInput(KEY_9) viewModel.deleteDigit() - assertEquals("3*√", viewModel.inputValue.value) + assertEquals("3*√", viewModel.input.value) assertEquals("3×√", viewModel.mainFlow.value.inputValue) } @@ -217,7 +217,7 @@ class MainViewModelTest { input.forEach { viewModel.processInput(it.toString()) } - assertEquals(output, viewModel.inputValue.value) + assertEquals(output, viewModel.input.value) assertEquals(outputDisplay, viewModel.mainFlow.value.inputValue) viewModel.clearInput() }