[부나] 뷰 챌린지 미션 1단계 제출합니다. #6

tmdgh1592 · 2023-09-27T05:03:39Z

안녕하세요 해시 #️⃣
코드 이해를 돕기 위해, 아래 간단한 설명을 첨부하였습니다.
리뷰 잘 부탁드립니다 😃

커스텀뷰

커스텀뷰는 크게 CanvasView / PaletteView / ColorScrollView 를 구현하였습니다.

CanvasView

실제로 터치를 통해 그리기가 가능한 뷰입니다.

PaletteView

PaletteView는 브러시의 굵기 설정, 색상 설정 이 가능한 뷰입니다.
Listener를 사용하여, PaletteView에서 브러시의 속성을 변경하면, CanvasView 에 반영되도록 구현하였습니다.
내부적으로 굵기 설정 뷰 / 색상 설정 뷰(ColorScrollView) 가 수직적으로 배치되어 있습니다.

ColorScrollView

ColorScrollView 는 ColorView 들이 수평적으로 배치되어 있는 형태로, 색상을 설정하기 위한 뷰입니다.

구현 화면

270864760-adf8c7fa-71b1-4c16-b57d-d549ea139d26

#FFA100 : CanvasView
#F75690 : PaletteView
#000080 : ColorScrollView

EmilyCh0

부나! 1단계 미션 고생하셨습니다 👍 코멘트 확인하고 다시 리뷰 요청해 주세요!

EmilyCh0 · 2023-09-27T08:35:06Z

app/src/main/java/woowacourse/paint/view/PaletteView.kt

+class PaletteView : LinearLayout {
+    constructor(context: Context) : super(context)
+    constructor(context: Context, attrs: AttributeSet) : super(context, attrs)


@JvmOverloads를 사용하는 건 어떤가요?

@JvmOverloads 를 사용하면 값을 지정하지 않은 경우에 대해 부모 생성자를 호출하는 것이 아닌, 추가로 생성된 자신의 생성자에 기본 값을 전달하여 호출합니다.
즉, 마치 정상적으로 오버로드된 것처럼 보이지만 개발자의 의도대로 동작하지 않을 수 있다는 문제가 있습니다.

View나 LinearLayout의 경우, 부모에서 defStyleAttr에서 0을 전달하기 때문에 해당 애노테이션을 사용해도 문제가 되지 않지만,
다른 위젯뷰들의 경우 별도로 스타일을 지정하지 않으면 부모 생성자에서 기본으로 지정하는 스타일이 있습니다 : )
그래서 저는 이런 문제들을 생각하지 않으려고 애초에 생성자를 직접 오버로드 해주는 것을 선호합니다 😃

그렇군요! defStyleAttr에 디폴트 스타일이 지정되어 있는 뷰는 문제가 있을 수 있겠네요!! 👍

EmilyCh0 · 2023-09-27T08:55:18Z

app/src/main/java/woowacourse/paint/view/PathPainter.kt

+    fun lineTo(pointX: Float, pointY: Float) {
+        path.lineTo(pointX, pointY)
+    }


path.quadTo()를 사용하면 부드러운 곡선을 그릴 수 있습니다!

⬇️⬇️⬇️

이런 메서드가 있는 줄 몰랐네요 😲
직선을 그리는 lineTo()에 비해 훨씬 부드러워 보입니다!

(차이가 없어 보이는 건 제 악필 탓입니다..)

EmilyCh0 · 2023-09-27T14:09:33Z

app/src/main/java/woowacourse/paint/view/ColorScrollView.kt

+    private val paletteColorViews = LinearLayout(context).apply {
+        layoutParams = LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT)
+        orientation = LinearLayout.HORIZONTAL
+    }
+
+    init {
+        addView(paletteColorViews)
+    }


동적으로 색상 뷰를 추가할 수 있다는 점은 장점이지만 너무 장풍 쏘는 구조가 되는 거 아닌가 싶어요.. 부나는 어떻게 생각하시나요??

ConstaintLayout → palette_view(LinearLayout) → ColorScollView(HorizontalScrollView) → paletteColorViews(LinearLayout) → ColorView
이런식으로 레이아웃이 구성되어 있는데 사실 커스텀뷰를 사용하지 않았다면 ConstaintLayout 내에서 처리할 수 있을 것 같고, onColorSelected를 ColorView까지 전달 전달 하지도 않을 것 같아서요!

말씀해주신 것처럼, 현재 구조를 계층적으로 나타내면 아래와 같습니다!

ConstraintLayout (1)

PaletteView (2)

ColorScrollView (3)

LinearLayout (4)

ColorView (5)

ColorView

ColorView

...

만약 커스텀 뷰를 사용하지 않았다면 아래와 같습니다.

ConstraintLayout (1)

ScrollView (2)

LinearLayout (3)

ColorView (4)

ColorView

ColorView

결국 커스텀뷰를 사용하나, 사용하지 않으나 실제로 깊이는 1 밖에 차이가 없어요!
(1 Depth 도 누군가는 크다고 느낄 수는 있겠지만요.. 😃)

커스텀뷰를 사용하여 xml에 PaletteView 하나만 있는 경우 vs 커스텀 뷰를 사용하지 않고 xml에서 직접 장풍 쏘는 경우
구조상 1 Depth가 늘어나고는 있지만, xml에서 시각적으로 봤을 땐 오히려 1 Depth 만 차지하고 있습니다.

커스텀뷰를 라이브러리로 배포하거나, 모듈화하여 팀 내 동료 개발자와 공유하거나, 여러 xml에서 재사용하는 관점에서 봤을 때
클라이언트 입장에서는 실제 Depth가 얼마인지보다는 xml에서 얼마나 더 편리하게 사용할 수 있는가에 집중할 것 같습니다.
더군다나 1Depth 밖에 차이가 나지 않구요 : )

또한, 색상, 굵기 변경과 같은 이벤트를 interface로 제공하고 있기 때문에 클라이언트 입장에서도 불편함은 크게 느끼지 못할 것 같습니다.

하지만 리뷰해주신 것처럼 ColorView의 클릭 이벤트를 부모 뷰로 전달, 전달, 전달.. 하고 있기 때문에 개발자의 유지보수 측면에서는 불편할 것 같아요!
(요리사가 힘들면 먹는 사람이 만족하듯이, 개발자가 힘들면 사용하는 사람이 만족할 것이라는 생각으로 코드를 작성했습니다. 😅)

이보다 더 좋은 구조가 있다면 그것을 따르겠지만, 지금의 저의 지식에서는 이 방법이 최선이라고 느껴집니다.

👍 부나 의견에 공감합니다. 여러 이점을 고려했을 때 depth가 늘어가는 것을 감수하고 커스텀뷰를 사용할 수 있다고 생각해요! 다만 저는 여기저기서 재사용되거나 CanvasView처럼 xml로는 처리할 수 없는 경우에만 커스텀뷰 사용하고 나머지는 xml로 처리하는 것을 선호합니다ㅎㅎ 커스텀뷰가 겹겹이 있으니까 어떻게 동작하는건지 코드를 파악하기가 어렵더라고요.. 특히나 addView로 뷰를 추가해주다 보니 뷰가 어떻게 구성되는지 파악하기가 어려웠던 것 같아요. (커스텀뷰가 익숙하지 않아서 그런걸수도..😅)

저도 해시 의견에 공감합니다 👍
반드시 커스텀 뷰로 구현해야만 하는 상황 에만 커스텀 뷰를 사용한다 라는 자신만의 기준이 있을 수 있겠네요!

EmilyCh0 · 2023-09-27T14:11:46Z

app/src/main/java/woowacourse/paint/view/ColorScrollView.kt

+    }
+
+    private fun resetPaletteColorViews() {
+        paletteColorViews.removeAllViews()


removeAllViews()는 왜 하는건지 궁금해요!

removeAllViews() 를 호출할 필요가 없네요 😅
해당 함수가 단 한 번만 호출될텐데.. 제 착각으로 추가된 코드입니다.

EmilyCh0 · 2023-09-27T14:14:06Z

app/src/main/java/woowacourse/paint/view/ColorScrollView.kt

+        fun create(
+            context: Context,
+            onColorSelected: (PaletteColor) -> Unit,
+        ): ColorScrollView = ColorScrollView(context).also { view ->
+            view.onColorSelected = onColorSelected
+        }


팩토리 함수로 프로퍼티 설정하는거 좋네요. 저도 가져다 써야겠어요ㅋㅋ

EmilyCh0 · 2023-09-27T14:19:51Z

app/src/main/java/woowacourse/paint/view/ColorView.kt

+    override fun onDraw(canvas: Canvas) {
+        super.onDraw(canvas)
+        canvas.drawRect(
+            HORIZONTAL_MARGIN,
+            TOP_MARGIN,
+            width.toFloat() - HORIZONTAL_MARGIN,
+            height.toFloat(),
+            paint,
+        )
+    }


와우 팔레트 색상을 다 네모네모 그려주고 있군요

네모네모하게 그려주고 있습니다~~

EmilyCh0 · 2023-09-27T14:23:27Z

app/src/main/java/woowacourse/paint/view/ColorView.kt

+        private const val COLOR_VIEW_SIZE = 180
+        private const val TOP_MARGIN = 30F
+        private const val HORIZONTAL_MARGIN = 20F


사이즈를 값으로 지정해주는 것도 어쩔 수 없는건지 궁금하네요..! 저도 부나랑 비슷하게 색상 뷰를 추가하는 식으로 시도해봤는데 커스텀뷰에 익숙하지 않아서인지 이쁘게 배치하는 것이 쉽지 않더라구요! constraintDimensionRatio랑 weight를 사용할 때처럼 비율에 딱 맞게 배치하고 싶었는데 말이죠.. 🤔

+) 미션 예시 화면처럼 사이즈 정해두는 것이 아니라 양끝에 딱 맞게 늘린 배치가 되는 경우를 생각해볼 수 있을 것 같아요! 부나가 구현한 것처럼 좌우 스크롤 하는 형태하면 해당사항이 없는 얘기라 고민만 해보셔도 좋을 것 같아요!

아래 코드를 참고해보세요!
저는 ColorView의 크기를 부모 뷰의 높이에 맞추도록 변경하였습니다.

override fun onMeasure(widthMeasureSpec: Int, heightMeasureSpec: Int) { super.onMeasure(widthMeasureSpec, heightMeasureSpec) val measuredSize = MeasureSpec.getSize(heightMeasureSpec) setMeasuredDimension(measuredSize, measuredSize) }

👍 부모 뷰의 height를 지정해주고 이걸 이용하는 것도 좋네요

EmilyCh0 · 2023-09-27T14:32:14Z

app/src/main/java/woowacourse/paint/view/PathPainter.kt

+    fun setPaintColor(paletteColor: PaletteColor): PathPainter = copy(
+        path = Path(),
+        paint = updatePaint(paintColor = paletteColor.color),
+    )
+
+    fun setThickness(thickness: Float): PathPainter = copy(
+        path = Path(),
+        paint = updatePaint(thickness = thickness),
+    )


모든 값을 바꿔주는 것 같은데 copy하는 의미가 있나요?

해시 말이 맞습니다!
코드에서 copy를 사용하나, 생성자로 객체를 다시 생성해주나 동일하기 때문에 지금은 둘 중 어떤 것을 선택하든 큰 의미는 없습니다.
다만, 이후에 유지되어야 하는 프로퍼티가 추가된다면 copy() 를 선택하는 것이 의미가 있을 것이고,
유지되지 않아야 하는 프로퍼티가 추가된다면 생성자 를 사용하는 것이 의미가 있을 것 같습니다!

아직은 어떻게 될 지 몰라서 여기서는 큰 의미를 담지 않고 copy를 사용하였습니다 : )

EmilyCh0 · 2023-09-27T14:36:42Z

app/src/main/java/woowacourse/paint/view/PathPainter.kt

+): Paint = apply {
+    isAntiAlias = true
+    style = Paint.Style.STROKE
+    strokeJoin = Paint.Join.ROUND


strokeJoin은 어떤 옵션인가요??

strokeJoin은 선이 연결될 때 보이는 모서리를 어떻게 표현할지 정의하는 속성입니다.
Round를 지정해주었기 때문에 모서리가 둥글게 보입니다 😃

EmilyCh0 · 2023-09-28T01:46:44Z

app/src/main/java/woowacourse/paint/view/CanvasView.kt

+            MotionEvent.ACTION_MOVE -> pathPainter.lineTo(pointX, pointY)
+        }
+        invalidate()
+        return true


return true가 무엇을 의미하나요??

Touch 이후로 이벤트를 전달하지 않겠다는 의미입니다.

EmilyCh0

피드백 반영된 것 확인했습니다! 더 리뷰 드릴게 없어서 approve 할게요. 추가 댓글만 한번 확인해주세요 😄

tmdgh1592 added 8 commits September 26, 2023 23:43

feat: 화면에 곡선을 그릴 수 있는 기능 구현

93c83e3

feat: 색상을 나타내는 뷰 구현

da6faf2

feat: ColorScrollView 구현

6d7581f

feat: 팔레트(색상/두께) 화면 구현

d956fa9

feat: 메인 액티비티에 캔버스, 팔레트 화면 구현

9b1f379

feat: 페인터의 기본 두께 비율을 0.1로 시작하도록 변경

5c695d7

feat: 화면에 클릭시 점 찍히는 기능 구현

71672ae

feat: 굵기, 색상 변경이 기록되는 기능 구현

3c44fa6

tmdgh1592 requested a review from EmilyCh0 September 27, 2023 05:03

tmdgh1592 self-assigned this Sep 27, 2023

EmilyCh0 requested changes Sep 28, 2023

View reviewed changes

tmdgh1592 added 5 commits September 28, 2023 12:34

refactor: 팔레트 색상 뷰들을 모두 지워주는 메서드 제거

e106c98

refactor: initPaletteColorViews() 메서드명을 setupPaletteColorViews()로 변경

ad3e8c6

refactor: ColorView의 paletteColor 프로퍼티를 private으로 변경

527f018

feat: 직선 대신 곡선을 그리도록 변경

f50a467

feat: ColorView의 크기를 ColorScrollView의 높이로 고정

e134bed

tmdgh1592 requested a review from EmilyCh0 September 28, 2023 05:03

EmilyCh0 approved these changes Sep 28, 2023

View reviewed changes

EmilyCh0 merged commit 457efc0 into woowacourse:tmdgh1592 Sep 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[부나] 뷰 챌린지 미션 1단계 제출합니다. #6

[부나] 뷰 챌린지 미션 1단계 제출합니다. #6

tmdgh1592 commented Sep 27, 2023

EmilyCh0 left a comment

EmilyCh0 Sep 27, 2023

tmdgh1592 Sep 28, 2023 •

edited

Loading

EmilyCh0 Sep 28, 2023

EmilyCh0 Sep 27, 2023

tmdgh1592 Sep 28, 2023

tmdgh1592 Sep 28, 2023 •

edited

Loading

EmilyCh0 Sep 27, 2023

tmdgh1592 Sep 28, 2023 •

edited

Loading

tmdgh1592 Sep 28, 2023

tmdgh1592 Sep 28, 2023

EmilyCh0 Sep 28, 2023

tmdgh1592 Sep 28, 2023

EmilyCh0 Sep 27, 2023

tmdgh1592 Sep 28, 2023

EmilyCh0 Sep 27, 2023

EmilyCh0 Sep 27, 2023

tmdgh1592 Sep 28, 2023

EmilyCh0 Sep 27, 2023 •

edited

Loading

tmdgh1592 Sep 28, 2023

EmilyCh0 Sep 28, 2023

EmilyCh0 Sep 27, 2023

tmdgh1592 Sep 28, 2023 •

edited

Loading

EmilyCh0 Sep 27, 2023

tmdgh1592 Sep 28, 2023

EmilyCh0 Sep 28, 2023

tmdgh1592 Sep 28, 2023

EmilyCh0 left a comment

[부나] 뷰 챌린지 미션 1단계 제출합니다. #6

[부나] 뷰 챌린지 미션 1단계 제출합니다. #6

Conversation

tmdgh1592 commented Sep 27, 2023

커스텀뷰

CanvasView

PaletteView

ColorScrollView

구현 화면

EmilyCh0 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmdgh1592 Sep 28, 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

tmdgh1592 Sep 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmdgh1592 Sep 28, 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

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

EmilyCh0 Sep 27, 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

tmdgh1592 Sep 28, 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

EmilyCh0 left a comment

Choose a reason for hiding this comment

tmdgh1592 Sep 28, 2023 •

edited

Loading

tmdgh1592 Sep 28, 2023 •

edited

Loading

tmdgh1592 Sep 28, 2023 •

edited

Loading

EmilyCh0 Sep 27, 2023 •

edited

Loading

tmdgh1592 Sep 28, 2023 •

edited

Loading