Skip to content

HomeWork07_Canvas#125

Open
Polina-Tarl wants to merge 6 commits intoOtus-Android:masterfrom
Polina-Tarl:master
Open

HomeWork07_Canvas#125
Polina-Tarl wants to merge 6 commits intoOtus-Android:masterfrom
Polina-Tarl:master

Conversation

@Polina-Tarl
Copy link

No description provided.

paint.color = colorList[colorIndex % colorList.size]
colorIndex++

val path = Path()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

стоит избегать создания объектов внутри onDraw (он вызывается 60 раз в секунду, это нагрузка на сборщик мусора), вместо этого можно вынести его создание из метода на уровень поля класса и переиспользовать его, сбрасывая перед каждым использованием:
path.reset() а уже затем path.moveTo и тд

class CirclePaymentCategoryFragment : Fragment(R.layout.fragment_circle_payment_category) {

private val pieChartItems = listOf(
PieEntry(5000f, "Кошка"),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

по условиям задачу данные нужно подгрузить из payload.json
в нем уже есть часть данных, можно добавить туда еще, если не хватает дефолтных, он лежит вот тут app/src/main/res/raw/payload.json

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не могла раньше найти) Спасибо!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

после реализации чтения из джсона можно это удалить

}

@SuppressLint("ClickableViewAccessibility")
override fun onTouchEvent(event: MotionEvent): Boolean {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

по условию задачи нужно не просто отреагировать обновлением UI внутри себя, а вызвать колбек с названием категории, на которую нажали, т.е нужно вынести логику с обновлением из onTouchEvent наружу и можно будет прописать любое желаемое поведение (обновить totalText, показать тост или перейти в детализацию по категории, например), сейчас это захардкожено внутри onTouchEvent и увеличивает связность кода

val desiredWidth = 600
val desiredHeight = 400

val width = when (widthMode) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можно упросить через resolveSize(desiredWidth, widthMeasureSpec), он под капотом делает аналогичную обработку, тогда и widthMode ну нужен будет и код станет сильно короче, аналогично с высотой

val legendStartY = 10f
var legendOffsetY = 0f
var legendColorIndex = 0
categoryData.keys.forEach { category ->

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

если нам нужен счетчик, то вместо отдельного legendColorIndex и forEach можно использовать forEachIndexed, нет лишних переменных, код чище, аналогично в следующем forEach

add(context.getColor(R.color.yellow))
}

private var savedColorIndex = 0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не очень понятно зачем на эта переменная, нигде не используется, только сохраняем в бандл и обратно вытаскиваем, осталась от другой реализации?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Да, забыла почистить)

}
private val rectF = RectF()
private val paint = Paint()
private var savedStartAngle = 0f

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

аналогично с savedColorIndex неясно зачем нужна, не участвует по сути в логике, только в сохранении стейта

return false
}

override fun onSaveInstanceState(): Parcelable? {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

totalText не сохраняется, при повороте экрана потеряем значение

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Попыталась добавить в сохранение, но визуально это не сработало. Причина в том, что view почему то инититься дважды - это видно по логам внутри.
image

Я не знаю почему так происходит :/ Вы могли бы посмотреть?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

оставил про это коммент в MainActivity

val modeHeight = MeasureSpec.getMode(heightMeasureSpec)

val desiredSize = 300
val finalWidth = when (modeWidth) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тут аналогично с onMeasure в GraphicPaymentCategoryView

super.onCreate(savedInstanceState)
setContentView(R.layout.activity_main)

supportFragmentManager

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

скорее всего вот тут проблема в дублировании вью, потому что при повороте будет создаваться новый фрагмент плюсом к восстановленному, нужно добавить проверку на
savedInstanceState == null

return false
}

override fun onSaveInstanceState(): Parcelable? {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

оставил про это коммент в MainActivity

class CirclePaymentCategoryFragment : Fragment(R.layout.fragment_circle_payment_category) {

private val pieChartItems = listOf(
PieEntry(5000f, "Кошка"),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

после реализации чтения из джсона можно это удалить


class GraphicPaymentCategoryFragment : Fragment(R.layout.fragment_graphic_payment_category) {

private val data = mapOf(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

это тоже видимо уже можно удалить, перешли на джсон

}

override fun onMeasure(widthMeasureSpec: Int, heightMeasureSpec: Int) {
val desiredWidth = 600

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ширина и высота захардкожена в пикселях, лучше задавать в дп и потом пересчитывать в пиксели например через displayMetrics, чтобы одинаково смотрелась на разных экранах, и вынести это в константы

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants