[아크] 뷰 챌린지 미션 2단계 제출합니다. #27

re4rk · 2023-10-06T06:55:50Z

안녕하세요 수달! 이번 미션도 잘부탁드립니다!
선택 요구 사항들은 미션 1단계에서 이미 진행한 내용들이라 기능 요구 사항 들만 체크해주시면 됩니다!

변경점

1. Brush의 paint와 path를 생성자에서 내부 프로퍼티로 이동

path는 당장 밖에서 넣어주는 것보다는 캡슐화를 위해서 숨겼습니다!
default paint를 적용하기 위해서 내부 프로퍼티로 이동하고 밖에서 접근 가능하도록 변경하였습니다!

2. Brush의 인터페이스화

이번 PR에서 가장 큰 변화점은 Draw 유형을 Brush가 Class에서 Interface로 변경되었다는 것입니다!
BrushLine과 BrushFigure를 사용해서 코드 중복부분을 제거하였습니다!

classDiagram
Brush <|-- BrushPen
Brush <|-- BrushEraser
Brush <|-- BrushCircle
Brush <|-- BrushRect

BrushPen <|-- BrushLine
BrushEraser <|-- BrushLine
BrushCircle <|-- BrushFigure
BrushRect <|-- BrushFigure

Brush : *interface
Brush : paint
Brush : start()
Brush : move()
Brush : drawOn()

BrushLine : *abstract

BrushFigure : addFigure()
BrushFigure : *abstract

기능 요구 사항

화면에 사각형, 원 모양을 그릴 수 있도록 구현한다.
- 사용자가 자유롭게 크기나 위치를 조정할 수 있다.
특정 영역의 요소를 지울 수 있는 지우개를 구현한다.
브러쉬(펜, 사각형, 원, 지우개) 종류를 자유롭게 선택해서 그릴 수 있다.

선택 요구 사항

전체 그림을 지우는 기능을 추가한다.
취소(undo), 재실행(redo)을 구현한다.

마지막 현재 코드를 작성하면서 노력하고 있는 점이라면 Code Change의 변화가 없도록 하는 것입니다!
그렇기에 당장은 불필요한 부분처럼 보이는 것이 있습니다!
예시로 다음과 같이 바로 point로 만들고 있지만 추후에 public으로 풀어서 바로 사용할 수 있도록 염두해두고 있습니다!
그래서 추후에는 x,y와 point로 모두 접근하는 방식을 사용하려고 합니다!

    override fun start(x: Float, y: Float, onCommit: () -> Unit) = start(Point(x, y), onCommit)

    private fun start(point: Point, onCommit: () -> Unit) {
        basePoint = point
        onCommit()
    }

이 구조에 대해서 수달은 어떻게 생각하는지 궁금합니다!

otter66

아크 고생 많으셨습니다!!
점박이 수달 재밌네요 감사합니다 ㅋㅋㅋ

그리고 step1에 추가로 남겼던 코멘트에 대한 아크의 의견도 궁금해요!!!
step2 PR에 추가로 남겨주면 좋을 것 같아요!

또한, step3도 함께 진행해주셔도 좋을 것 같습니다!

otter66 · 2023-10-07T04:25:57Z

app/src/main/java/woowacourse/paint/util/PointUtil.kt

+fun createRectFOf(point: Point, otherPoint: Point): RectF {
+    val minX = minOf(point.x, otherPoint.x)
+    val minY = minOf(point.y, otherPoint.y)
+    val maxX = maxOf(point.x, otherPoint.x)
+    val maxY = maxOf(point.y, otherPoint.y)
+    return RectF(minX, minY, maxX, maxY)
+}


꼭 Util로 빠져 나와야 했을까요??
RectF를 필요로 하는 객체 등 객체에 포함시킬 수는 없을까요?🤔

Point로 이동했습니다!

otter66 · 2023-10-07T04:33:19Z

app/src/main/java/woowacourse/paint/model/Brushes.kt

+    operator fun <T : Brush> plusAssign(brush: T) {
        history.add(brush)
        undoHistory.clear()
    }


여기서 꼭 제네릭을 써야하는 이유가 있었나요?
이전처럼 brush를 받아오는 것과 어떤 차이가 있나요? 🤔

리팩토링 단계에서 고쳤던 것인데 반영이 되지 않았었네요! 수정했습니다!

app/src/main/res/values/strings.xml

otter66 · 2023-10-07T04:47:58Z

app/src/main/java/woowacourse/paint/customVeiw/PaintingPaper.kt

-    var onEraseModeChangeListener: (Boolean) -> Unit = {}
+    init {
+        background = ColorDrawable(Color.WHITE)
+        setLayerType(LAYER_TYPE_HARDWARE, null)


setLayerType(LAYER_TYPE_HARDWARE, null) 이 구문이 없으면 왜 까맣게 나올까요 🤔?

하드웨어 가속을 사용하지 않는다면 성능상의 이슈로 검정색으로 처리하게 됩니다! 따라서 투명하게 만들어서 원하는 동작을 하게 만들라면 하드웨어 가속을 사용해야합니다!

app/src/main/java/woowacourse/paint/model/Brush.kt

otter66 · 2023-10-07T05:15:55Z

app/src/main/java/woowacourse/paint/model/Brush.kt


-    fun start(x: Float, y: Float, onCommit: () -> Unit) = start(Point(x, y), onCommit)
+    fun start(x: Float, y: Float, onCommit: () -> Unit = {})


start가 어떤 역할을 하는지 잘 모르겠습니다 🥲
메서드명이 조금 더 명확하면 좋겠습니다. 마찬가지로, 이 메서드는 꼭 고차함수여야 하나요? 🤔
고차함수를 사용할 경우에는 어떠한 이벤트에 대해 해당 액션을 수행할 것인지 표기가 되어야한다고 생각합니다.

Brush를 인터페이스화 시키기 위해서 start로 하였습니다!
startDrawing과 continueDrawing으로 변경하였습니다!

선언형으로 프로그래밍하기 위해서 고차함수를 활용했습니다!
고차 함수 모두 onSuccess로 어떤 액션에서 수행할지 변경하였습니다!

otter66 · 2023-10-07T05:23:18Z

app/src/main/java/woowacourse/paint/model/BrushFigure.kt

+    private fun start(point: Point, onCommit: () -> Unit) {
+        basePoint = point
+        onCommit()
+    }


x, y, point로 모두 접근하는 방식이란 어떤 것인가요?
또, 왜 그렇게 해야하나요?
잘 이해가 안되어서 조금만 더 자세하게 설명해주실 수 있을까요?

밖에서는 point로 접근하게 하고 싶지 않았습니다!
예시로 (x,y)와 point로 모두 접근하는 방식이란 RectF과 (left, top, right, bottom)의 관계를 들 수 있을 것같습니다!

re4rk · 2023-10-10T12:26:08Z

리뷰를 반영하면서 Configuration Change 대응을 추가하였습니다!
요청하신 step1 내용은 전 PR에 코멘트에 대한 답글을 달아놨습니다!

otter66

아크 이번 리뷰 반영 수고 많으셨습니다!
추가로 제가 미션을 더 진행하며 알게 된 부분들에 대해서 추가로 코멘트 남겼습니다!
또한, step1 PR에도 코멘트 추가로 달아두었습니다!
그리고 step3 미션을 자세히 읽어 보았는데, 코드 체인지가 꽤 많을 것 같아요.
이번 리뷰만 빠르게 반영 후 머지하도록 하겠습니다. 그 후에 step3으로 넘어가보는건 어떨까요!?

otter66 · 2023-10-11T01:53:36Z

app/src/main/java/woowacourse/paint/model/BrushEraser.kt

현재는 전체 지우기를 한 후에 되돌리기(undo)가 안되는데요.
이 부분도 어떻게 구현할 수 있을지 고민해보면 좋을 것 같아요!

전체 지우기를 반영하지 않은 것은 의도한 것이였지만 재밌어보여서 반영해봤습니다.

otter66 · 2023-10-11T01:57:29Z

app/src/main/java/woowacourse/paint/ui/MainViewModel.kt

brushHistory를 MainViewModel이 들고 있습니다.
하지만, redo, undo, clear와 같이 brushHistory에 대한 관리는 외부(Paintingpager)에서 이루어지고 있습니다.
저는 이 부분이 어색하다고 느껴집니다.
개선해볼 수 없을까요?

ViewModel은 Domain과 View의 중간 다리의 역할과 데이터의 저장을 맡습니다.
이 때 Domain과의 소통이 필요한 작업이 필요하다면 말씀하신대로 외부에서 이루어지는 것이 어색할 수 있다고 생각합니다.

잘못생각한 부분이 있어서 개선하겠습니다!

otter66 · 2023-10-11T02:32:59Z

app/src/main/java/woowacourse/paint/ui/MainActivity.kt

+            when (checkedId) {
+                R.id.rbPen -> paintingPaper.brushShape = BrushShape.LINE
+                R.id.rbRect -> paintingPaper.brushShape = BrushShape.RECT
+                R.id.rbCircle -> paintingPaper.brushShape = BrushShape.CIRCLE
+                R.id.rbEraser -> paintingPaper.brushShape = BrushShape.ERASER
+            }


현재 구조를 이용해 when문을 생략할 수 있을 것 같아요!

otter66 · 2023-10-11T02:35:17Z

app/src/main/java/woowacourse/paint/model/BrushRect.kt

+class BrushRect : BrushFigure() {
+    override fun addFigure(point: Point) {
+        path.addRect(basePoint.getRectF(point), Path.Direction.CCW)
+    }
+}


Path.Direction.CCW는 어떤 것일까요?

stackoverflow
그리는 방향성을 설정합니다!

otter66 · 2023-10-11T02:52:20Z

app/src/main/java/woowacourse/paint/model/BrushHistory.kt

@@ -2,7 +2,7 @@ package woowacourse.paint.model

 import android.graphics.Canvas

-class Brushes() {
+class BrushHistory {


레아가 슬랙에 올려주신 글이 있습니다!
https://refactoring.guru/design-patterns/memento
이 링크를 올려주셨었는데요! (사실 저도 잘 이해하지는 못했습니다)
해당 글의 스레드에 레아가 좀 더 잘 요약해서 올려주셨어요!
스레드를 참고해보면 구조를 조금 더 개선해볼 수 있을 것 같아요!

otter66 · 2023-10-11T03:03:30Z

app/src/main/java/woowacourse/paint/model/BrushLine.kt

+    override fun continueDrawing(x: Float, y: Float, onSuccess: () -> Unit) =
+        continueDrawing(Point(x, y), onSuccess)
+
+    private fun continueDrawing(point: Point, onSuccess: () -> Unit = {}) {
+        if (isDrawable(point)) {
+            path.lineTo(point.x, point.y)
+            lastPoint = point
+            onSuccess()
+        }
+    }


사실 현재 이 구조가 잘 이해 되지는 않네요🥲
재귀함수도 있고, 고차함수도 있고, 꼬리에 꼬리를 무는 느낌이랄까요...
30분 정도 이 부분만 들여다본 것 같은데, 코멘트를 뭐라고 남겨야할지 모르겠습니다🥲
좀 더 간략하게 바꿀 수 있다면 도전해보시면 좋을 것 같아요.

구현체가 아닌 인터페이스를 바라보는 것이 어떨까요?
onSuccess는 계속 그리기가 성공할 때 실행하는 함수 입니다.

continueDrawing(x: Float, y: Float, onSuccess: () -> Unit)

코루틴을 배우기전 미션에서도 enqueue을 위해 사용을 했었고 카카오 SDK와 안드로이드 내부에서 사용하는 구조기에 적용을 해보았습니다.
많이 사용하는 방법이기에 이해가 되지 않는다는 이유로 구조를 바꾸는 것은 적합한 이유가 아니라고 생각이 듭니다.

otter66 · 2023-10-11T03:07:48Z

app/src/main/java/woowacourse/paint/model/Brush.kt

+        fun from(shape: BrushShape): Brush = when (shape) {
+            BrushShape.LINE -> BrushPen()
+            BrushShape.CIRCLE -> BrushCircle()
+            BrushShape.RECT -> BrushRect()
+            BrushShape.ERASER -> BrushEraser()
+        }


이 부분도 when을 안쓰고 할 수 있을 것 같아요!

otter66 · 2023-10-12T09:00:02Z

아크 수고 많으셨습니다!!
화면 회전에도 그림이 살아남도록 하는 방법이 인상적이었어요!
그 외에도 아크를 리뷰하며 여러모로 배울 수 있어서 좋았습니다!
step3 고고고!!!

re4rk added 15 commits October 5, 2023 17:53

feat: 사용할 버튼들 추가

62a04a7

feat: 다양한 모양 대응을 위해 Brush 인터페이스화

4ea28c8

feat: 직사각형과 원을 만들 수 있는 Brush 추가

2fb9918

refactor: 중복기능 util로 추가

a0e2bac

refactor: 지우개모드 개선

7f40cec

refactor: 따로 존재하는 지우개모드 Brush로 통합

2319b5e

refactor: 도화지 클래스에서 불필요한 함수 제거

c1d637f

refactor: 도화지 클래스 코드 길이 축소

680555a

refactor: 중복이 많은 setUpView 스코프 함수 적용

f5564ec

refactor: Brush의 path를 캡슐화

848ce21

refactor: 지우개와 펜의 중복 부분 제거

d38a191

refactor: 직사각형과 원의 중복 코드 제거

c22efdf

refactor: 그림에 필요한 paint 안으로 이동

50fada5

refactor: Brush 캡슐화

46e30f4

refactor: 멤버 변수 직관적인 네이밍으로 변경

f8cf6a2

otter66 requested changes Oct 7, 2023

View reviewed changes

otter66 reviewed Oct 7, 2023

View reviewed changes

re4rk added 13 commits October 10, 2023 03:55

style: 사용하지 않는 문자열 정리

18296a4

refactor: 제네릭을 사용하지 않도록 변경

c0f867a

refactor: 고차 함수 인자로 의미에 맞게 변경

17c03cf

chore: 힐트 의존성 추가

6c588e2

refactor: brushes에서 BrushHistory로 변경

f8a101f

refactor: 프로젝트 힐트 적용

933a91a

fix: 현재 그리고 있는 brush 출력되게 변경

e55729c

chore: AAC ViewModel 사용

19fef66

feat: viewModel 적용

5bf9f6b

refactor: 리플렉션 제거

7dde0f7

refactor: RectF 유틸함수 제거

c3f8a0c

refactor: Brush 네이밍 변경

aab1d25

refactor: onClick을 바로 사용하도록 변경

d12e670

re4rk added 2 commits October 10, 2023 21:16

refactor: view 관련 로직 setUpView로 이동

580048e

refactor: 스코프 함수 특정 객체를 작업하는 곳에서는 with적용

536a3e5

otter66 requested changes Oct 11, 2023

View reviewed changes

re4rk added 4 commits October 11, 2023 15:57

refactor: memento패턴 적용

6ed2341

refactor: when절 삭제

88f450c

chore: Parcelize를 위한 그래들 추가

1257f21

refactor: ViewModel 제거

84ee2da

otter66 approved these changes Oct 12, 2023

View reviewed changes

otter66 merged commit 701b3ec into woowacourse:re4rk Oct 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[아크] 뷰 챌린지 미션 2단계 제출합니다. #27

[아크] 뷰 챌린지 미션 2단계 제출합니다. #27

re4rk commented Oct 6, 2023

otter66 left a comment •

edited

Loading

otter66 Oct 7, 2023

re4rk Oct 10, 2023

otter66 Oct 7, 2023

re4rk Oct 9, 2023

otter66 Oct 7, 2023

re4rk Oct 10, 2023

otter66 Oct 7, 2023

re4rk Oct 10, 2023

otter66 Oct 7, 2023

re4rk Oct 10, 2023

re4rk commented Oct 10, 2023

otter66 left a comment

otter66 Oct 11, 2023

re4rk Oct 11, 2023

otter66 Oct 11, 2023

re4rk Oct 11, 2023 •

edited

Loading

re4rk Oct 11, 2023

otter66 Oct 11, 2023

otter66 Oct 11, 2023

re4rk Oct 11, 2023 •

edited

Loading

otter66 Oct 11, 2023

otter66 Oct 11, 2023

re4rk Oct 11, 2023

otter66 Oct 11, 2023

otter66 commented Oct 12, 2023


		fun start(x: Float, y: Float, onCommit: () -> Unit) = start(Point(x, y), onCommit)
		fun start(x: Float, y: Float, onCommit: () -> Unit = {})

[아크] 뷰 챌린지 미션 2단계 제출합니다. #27

[아크] 뷰 챌린지 미션 2단계 제출합니다. #27

Conversation

re4rk commented Oct 6, 2023

변경점

1. Brush의 paint와 path를 생성자에서 내부 프로퍼티로 이동

2. Brush의 인터페이스화

기능 요구 사항

선택 요구 사항

otter66 left a comment • 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

Choose a reason for hiding this comment

re4rk commented Oct 10, 2023

otter66 left a 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

re4rk Oct 11, 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

re4rk Oct 11, 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

otter66 commented Oct 12, 2023

otter66 left a comment •

edited

Loading

re4rk Oct 11, 2023 •

edited

Loading

re4rk Oct 11, 2023 •

edited

Loading