The android-calculator from kridan-coder

Ревью #1

Лишние комментарии лучше нужно убирать

Android-Calculator/app/src/main/java/com/example/calculator/MainActivity.kt

Lines 15 to 24 in 19f4868

 /* private lateinit var button0: Button 

  private lateinit var button1: Button 

  private lateinit var button2: Button 

  private lateinit var button3: Button 

  private lateinit var button4: Button 

  private lateinit var button5: Button 

  private lateinit var button6: Button 

  private lateinit var button7: Button 

  private lateinit var button8: Button 

  private lateinit var button9: Button*/

Эти переменные должны быть private, потому что используются только внутри класса активити

Android-Calculator/app/src/main/java/com/example/calculator/MainActivity.kt

Lines 26 to 30 in 19f4868

var oldValue = "0"

var newValue = "0"

var currValue = "0"

var operation = "nothing"

var gotErr = false

findViewById конечно жизнеспособный вариант, но порождает кучу лишнего кода. Куда проще и лучше использовать ViewBinding

Android-Calculator/app/src/main/java/com/example/calculator/MainActivity.kt

Lines 62 to 72 in 19f4868

 displayCurr = findViewById(R.id.textViewDigits) 

 buttonPlus = findViewById(R.id.button_plus) 

 buttonMinus = findViewById(R.id.button_minus) 

 buttonMultiply = findViewById(R.id.button_multiply) 

 buttonDivide = findViewById(R.id.button_divide) 

 buttonModulo = findViewById(R.id.button_percent) 

 buttonPlusMinus = findViewById(R.id.button_plusminus) 

 buttonEqual = findViewById(R.id.button_equal) 

 buttonAC = findViewById(R.id.button_AC) 

 buttonComma = findViewById(R.id.button_comma)

Операции лучше сделать с помощью enum. Когда операции нет в переменной можно хранить null. Так уйдет сравнение строк, которое чревато ошибками

Android-Calculator/app/src/main/java/com/example/calculator/MainActivity.kt

Line 29 in 19f4868

var operation = "nothing"
Мило, но этому к сожалению тут не место

Android-Calculator/app/src/main/java/com/example/calculator/MainActivity.kt

Line 170 in 19f4868

if (displayCurr.text == "H ITS" || displayCurr.text == "0")
Это условие делать трижды одну и ту же операцию. Вопрос, зачем оно тогда тут нужно? С условием ниже похожая история.

Android-Calculator/app/src/main/java/com/example/calculator/MainActivity.kt

Lines 172 to 176 in 19f4868

displayCurr.text = digit.toString()

if (digit == 1)

displayCurr.text = "$digit"

else

displayCurr.text = digit.toString()
Всю конструкцию с регуляркой можно записать проще и понятнее. А будь enum и того проще.

Android-Calculator/app/src/main/java/com/example/calculator/MainActivity.kt

Lines 202 to 204 in 19f4868

val regex = """[%/*\-+]""".toRegex()

//val charArray = currValue.toCharArray()

if (regex.matches(oldValue.toCharArray()[oldValue.length - 1].toString())) {

oldValue.lastOrNull()?.toString() in listOf("%", "/", "*", "-", "+")

Вывести Toast с сообщением об ошибке было бы быстрее, чем написать этот комментарий

Android-Calculator/app/src/main/java/com/example/calculator/MainActivity.kt

Lines 219 to 221 in 19f4868

} catch (E: Exception) {

// some kind of error i dont even want to handle. i spent too much for this calculator. i have TONS of other tasks

}
Точки с запятой в Kotlin не нужны. Кроме одного случая
Это можно записать куда проще

Android-Calculator/app/src/main/java/com/example/calculator/MainActivity.kt

Lines 98 to 104 in 19f4868

private fun isFloat(value:String): Boolean{

if (value.indexOf('.') != -1)

{

return true

}

return false

}

private fun isFloat(value:String) = value.indexOf('.') != -1

Эта цифра 8 так и просится в константу

Android-Calculator/app/src/main/java/com/example/calculator/MainActivity.kt

Line 178 in 19f4868

else if (displayCurr.text.length <= 8)
Использовать второй Constraint в верстке не нужно. Все это верстается в одном лайауте. Так только перегружаешь лишний раз экран
Ширину и высоту лучше не выносить в стили. Так теряется гибкость твоего стиля.

Android-Calculator/app/src/main/res/values/styles.xml

Lines 8 to 9 in 19f4868

<item name="android:layout_width">0dp</item>

<item name="android:layout_height">0dp</item>
Вот за это убивать надо. Я понимаю, что так код стал короче и проще обработка, но понятность резко потерялась. Установить клик быстро на несколько элементов можно с помощью Group

Android-Calculator/app/src/main/res/layout/activity_main.xml

Line 93 in 19f4868

android:onClick="digitButtonPressed"
Любые строковые значения (и цвета тоже) должны быть в ресурсах и использоваться оттуда. Неважно используешь ты их в коде или верстке.

Android-Calculator/app/src/main/res/layout/activity_main.xml

Line 64 in 19f4868

android:text="Calculator"
То что знаешь и используешь Guideline хорошо, но тут явно перебор с ними. Все это можно сделать просто с помощью обычных привязок и Chain
Лучше не оставлять в верстке элементов с дефолтными id. Теряется читаемость твоего кода.

Android-Calculator/app/src/main/res/layout/activity_main.xml

Line 15 in 19f4868

android:id="@+id/imageView2"
Вот так приложение выглядит в альбомной ориентации. Скорее всего всему бедой размер шрифта + Guideline

kridan-coder / android-calculator Goto Github PK

android-calculator's Introduction

Welcome aboard 👋

android-calculator's People

Watchers

android-calculator's Issues

Ревью #1

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	/* private lateinit var button0: Button
	private lateinit var button1: Button
	private lateinit var button2: Button
	private lateinit var button3: Button
	private lateinit var button4: Button
	private lateinit var button5: Button
	private lateinit var button6: Button
	private lateinit var button7: Button
	private lateinit var button8: Button
	private lateinit var button9: Button*/

	var oldValue = "0"
	var newValue = "0"
	var currValue = "0"
	var operation = "nothing"
	var gotErr = false

	displayCurr = findViewById(R.id.textViewDigits)

	buttonPlus = findViewById(R.id.button_plus)
	buttonMinus = findViewById(R.id.button_minus)
	buttonMultiply = findViewById(R.id.button_multiply)
	buttonDivide = findViewById(R.id.button_divide)
	buttonModulo = findViewById(R.id.button_percent)
	buttonPlusMinus = findViewById(R.id.button_plusminus)
	buttonEqual = findViewById(R.id.button_equal)
	buttonAC = findViewById(R.id.button_AC)
	buttonComma = findViewById(R.id.button_comma)

	displayCurr.text = digit.toString()
	if (digit == 1)
	displayCurr.text = "$digit"
	else
	displayCurr.text = digit.toString()

	val regex = """[%/*\-+]""".toRegex()
	//val charArray = currValue.toCharArray()
	if (regex.matches(oldValue.toCharArray()[oldValue.length - 1].toString())) {

	} catch (E: Exception) {
	// some kind of error i dont even want to handle. i spent too much for this calculator. i have TONS of other tasks
	}

	private fun isFloat(value:String): Boolean{
	if (value.indexOf('.') != -1)
	{
	return true
	}
	return false
	}

	<item name="android:layout_width">0dp</item>
	<item name="android:layout_height">0dp</item>