-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Alex] Step4 다형성을 적용한 리팩토링 #105
base: alex
Are you sure you want to change the base?
Changes from all commits
52a4fb8
a0acaa7
13f4b28
7ef57a4
432834e
522582e
191c437
f37334d
69229fe
9d61c70
c7597e4
bc31471
535e81c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,60 +1,41 @@ | ||
// | ||
// Rectangle.swift | ||
// ColoredRectangle.swift | ||
// DrawingApp | ||
// | ||
// Created by 송태환 on 2022/03/01. | ||
// | ||
|
||
import Foundation | ||
|
||
protocol RectangleBuildable { | ||
init(x: Double, y: Double, width: Double, height: Double) | ||
} | ||
typealias BackgroundColorControllable = BackgroundAdaptable & AlphaAdaptable | ||
|
||
class Rectangle: Shapable, Notifiable, Hashable { | ||
enum NotificationKey { | ||
case alpha | ||
case color | ||
} | ||
|
||
class ColoredRectangle: Shape, BackgroundColorControllable, Notifiable { | ||
// MARK: - Properties | ||
private(set) var backgroundColor: Color { | ||
didSet { | ||
self.notifyDidUpdated(key: .color, data: self.backgroundColor) | ||
self.notifyDidUpdated(key: .updated, data: self.backgroundColor) | ||
} | ||
} | ||
|
||
private(set) var alpha: Alpha { | ||
didSet { | ||
self.notifyDidUpdated(key: .alpha, data: self.alpha) | ||
self.notifyDidUpdated(key: .updated, data: self.alpha) | ||
} | ||
} | ||
|
||
let id: String | ||
let size: Size | ||
let origin: Point | ||
|
||
var diagonalPoint: Point { | ||
let maxX = self.origin.x + self.size.width | ||
let maxY = self.origin.y + self.size.height | ||
return Point(x: maxX, y: maxY) | ||
} | ||
|
||
Comment on lines
-33
to
-42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shape 클래스가 공통 데이터를 가지고 있고 ColoredRectangle 은 backgroundColor 와 alpha, ImageRectangle 은 image 와 alpha 데이터를 소유합니다. |
||
// MARK: - Initialisers | ||
init(id: String, origin: Point, size: Size, color: Color = .white, alpha: Alpha = .opaque) { | ||
self.id = id | ||
self.origin = origin | ||
self.size = size | ||
self.backgroundColor = color | ||
self.alpha = alpha | ||
super.init(id: id, origin: origin, size: size) | ||
} | ||
|
||
init(id: String, x: Double, y: Double, width: Double, height: Double, color: Color = .white, alpha: Alpha = .opaque) { | ||
self.id = id | ||
self.origin = Point(x: x, y: y) | ||
self.size = Size(width: width, height: height) | ||
self.backgroundColor = color | ||
let origin = Point(x: x, y: y) | ||
let size = Size(width: width, height: height) | ||
self.alpha = alpha | ||
self.backgroundColor = color | ||
super.init(id: id, origin: origin, size: size) | ||
} | ||
|
||
func convert<T: RectangleBuildable>(using Convertor: T.Type) -> T { | ||
|
@@ -79,7 +60,7 @@ class Rectangle: Shapable, Notifiable, Hashable { | |
} | ||
|
||
// MARK: - CustomStringConvertible Protocol | ||
extension Rectangle { | ||
extension ColoredRectangle: CustomStringConvertible { | ||
var description: String { | ||
return """ | ||
(\(self.id)), \(self.origin), \(self.size), \(self.backgroundColor), \(self.alpha) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,24 +7,52 @@ | |
|
||
import Foundation | ||
|
||
class ImageRectangle: Rectangle { | ||
private(set) var image: URL? | ||
typealias ImageControllable = ImageAdaptable & AlphaAdaptable | ||
|
||
class ImageRectangle: Shape, ImageControllable, Notifiable { | ||
private(set) var alpha: Alpha { | ||
didSet { | ||
self.notifyDidUpdated(key: .updated, data: self.alpha) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. didSet을 사용하는 경우는 이 값이 struct냐 class냐에 따라 동작이 다릅니다. |
||
} | ||
} | ||
|
||
private var imageURL: URL? | ||
|
||
var imagePath: String? { | ||
return self.imageURL?.path | ||
} | ||
Comment on lines
+19
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 캡슐화를 유지할 목적으로 getter 역시 메소드처럼 메모리가 동작한다고 해서 외부 객체가 직접 속성에 접근할 수 있게 하는 것보다 getter 를 두었습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Plane과 다르게 내부에서 저장을 위해서 생성되는 이런 타입들은 어쩔 수 없이 값을 가져가야 하는 경우가 생깁니다. |
||
|
||
init(id: String, origin: Point, size: Size, image: URL? = nil) { | ||
self.imageURL = image | ||
self.alpha = .opaque | ||
super.init(id: id, origin: origin, size: size) | ||
self.image = image | ||
} | ||
|
||
init(id: String, x: Double, y: Double, width: Double, height: Double, image: URL? = nil) { | ||
super.init(id: id, x: x, y: y, width: width, height: height) | ||
self.image = image | ||
let origin = Point(x: x, y: y) | ||
let size = Size(width: width, height: height) | ||
self.imageURL = image | ||
self.alpha = .opaque | ||
super.init(id: id, origin: origin, size: size) | ||
} | ||
|
||
func setImage(with url: URL) { | ||
self.image = url | ||
func setImagePath(with url: URL) { | ||
self.imageURL = url | ||
} | ||
|
||
override func notifyDidCreated() { | ||
func setAlpha(_ alpha: Alpha) { | ||
self.alpha = alpha | ||
} | ||
|
||
func convert<T: RectangleBuildable>(using Convertor: T.Type) -> T { | ||
return Convertor.init(x: self.origin.x, y: self.origin.y, width: self.size.width, height: self.size.height) | ||
} | ||
|
||
func notifyDidCreated() { | ||
NotificationCenter.default.post(name: .ImageRectangleModelDidCreated, object: self) | ||
} | ||
|
||
func notifyDidUpdated(key: NotificationKey, data: Any) { | ||
NotificationCenter.default.post(name: .RectangleModelDidUpdated, object: self, userInfo: [key: data]) | ||
Comment on lines
+51
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 음.. 이런 하위 타입들은 class여도 immutable 형태나 struct - let 형태로 값 타입으로 사용하는 경우도 많습니다 |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,10 +7,6 @@ | |
|
||
import Foundation | ||
|
||
protocol Notifiable: AnyObject { | ||
func notifyDidCreated() | ||
} | ||
|
||
class Plane { | ||
private var items = [String: Shapable]() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. String 인걸 보니 Id 값을 이용해서 키를 사용하는 것 같습니다 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 타입 자체의 비교를 고려한다면 딕셔너리보다는 배열로 바꾸는 것이 적합할 것 같습니다. |
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
// | ||
// AlphaAdaptable.swift | ||
// DrawingApp | ||
// | ||
// Created by 송태환 on 2022/03/18. | ||
// | ||
|
||
import Foundation | ||
|
||
protocol AlphaAdaptable { | ||
var alpha: Alpha { get } | ||
func setAlpha(_ alpha: Alpha) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
프로토콜로 대체하면서 이전에 없던 if 문과 타입케스팅 생겼습니다. 차라리 별도의 함수로 분리하는 것이 더 좋을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
맞아요. 고민스러운 부분이네요. 추상화를 했기 때문에 적절한 것을 찾지 못해서 어쩔 수 없죠.
그만큼 역할을 잘 나눈거죠. 이벤트 하나에서 역할에 따라 다르게 출력해야 하니까 생기는 현상이니까 괜찮습니다.
그래서 유즈케이스 같은 것을 두고 planeDidSelectItem가 발생하면, 거기서 분기해서 각기 다른 흐름을 만들기도 합니다