Skip to content
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

item50 적시에 방어적 복사본을 만들라 #105

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Limwin94
Copy link
Member

item50에 대한 초안을 작성해보았습니다.

기술적인 부분 보다는 방어적 복사본을 언제 만들고 왜 만들어야 하는지에 대한 내용이 중점인것 같았습니다.

  • inout도 다루는게 맞을까요?

Copy link
Member

@ehgud0670 ehgud0670 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!!! 💯💪
글 덕분에 struct 의 장점을 또 한번 알게 되네요 👍👍
그럼 제가 피드백 드린거 확인해주시고 반영해주세요~ (코드 위주로 살짝 수정해보았습니다)

Comment on lines +52 to +59
class Person {
var isHungry: Bool = true
}

let lin = Person()
let anotherPerson = lin

print(anotherPerson.isHungry) //true
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
class Person {
var isHungry: Bool = true
}
let lin = Person()
let anotherPerson = lin
print(anotherPerson.isHungry) //true
class Person {
var isHungry: Bool = true
}
let lin = Person()
let anotherPerson = lin
lin.isHungry = false
print(lin.isHungry) //false
print(anotherPerson.isHungry) //false

=> 이렇게 내용 추가하면 참조타입의 얕은 복사에 대해 더 설명될 것 같습니다. 👍 아래 코드 보고 추가해봤어요.

Copy link
Member

Choose a reason for hiding this comment

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

저도 이렇게 변경이 되어야 할 것 같아요 ㅎㅎ

Comment on lines +11 to +12
private var start: Date
private var end: Date
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private var start: Date
private var end: Date
private let start: Date
private let end: Date

=> 사소한건데 자바(final)처럼 let 으로 변경 부탁드립니다.

Comment on lines +43 to +46
Java에서는 Date가 가변 타입이므로 외부의 `start` 혹은 `end` 변수를 수정하게 될 경우 `period` 내부의 `start`, `end` 값 또한 변경됩니다.

하지만 Swift에서는 이러한 부작용이 적용되지 않습니다. Swift의 `값 타입(Value Type)`은 기본적으로 깊은 복사를 지원하고 Date가 `값 타입`인 struct(구조체)로 구성되어 있기 때문입니다.

Copy link
Member

Choose a reason for hiding this comment

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

와우 역시 Swift가 대단하네요. Struct라는 다른 수단이 있으니 말이죠 😎

Comment on lines +69 to +83
class Person: NSCopying {
func copy(with zone: NSZone? = nil) -> Any {
let copy = Person()
copy.isHungry = self.isHungry

return copy
}
}

let lin = Person()
let anotherPerson = lin.copy()
lin.isHungry = false

print(lin.isHungry) // false
print(anotherPerson.isHungry) // true
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
class Person: NSCopying {
func copy(with zone: NSZone? = nil) -> Any {
let copy = Person()
copy.isHungry = self.isHungry
return copy
}
}
let lin = Person()
let anotherPerson = lin.copy()
lin.isHungry = false
print(lin.isHungry) // false
print(anotherPerson.isHungry) // true
class Person: NSCopying {
var isHungry: Bool = true
func copy(with zone: NSZone? = nil) -> Any {
let copy = Person()
copy.isHungry = self.isHungry
return copy
}
}
let lin = Person()
let anotherPerson: Person = lin.copy() as! Person
lin.isHungry = false
print(lin.isHungry) // false
print(anotherPerson.isHungry) // true

해당 코드 컴파일 에러나서 (because of isHungry, Any) 에러안나도록 코드 수정하였습니다.

@ehgud0670 ehgud0670 removed the Jason label May 19, 2021
@ehgud0670
Copy link
Member

pr 내용을 보고 추가로 코멘트 드리면 inout은 안 다뤄도 될 것 같습니다!
자바 코드를 보면 객체의 내부로 입력인수로서 인수를 넘기기만 하지 출력인수, 즉 inout은 사용하지 않네요.

Copy link
Member

@delmaSong delmaSong left a comment

Choose a reason for hiding this comment

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

덕분에 NSCopying에 대해 알아갑니다 ㅎㅎ Jason이 제안해주신대로 수정부탁드립니다!
그리고 REAMDE도 수정해주셔요

Comment on lines +52 to +59
class Person {
var isHungry: Bool = true
}

let lin = Person()
let anotherPerson = lin

print(anotherPerson.isHungry) //true
Copy link
Member

Choose a reason for hiding this comment

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

저도 이렇게 변경이 되어야 할 것 같아요 ㅎㅎ

@delmaSong delmaSong removed the Delma label May 30, 2021
Copy link
Member

@dev-Lena dev-Lena left a comment

Choose a reason for hiding this comment

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

방어적 복사에 대한 글 잘 읽었습니다. 델마와 제이슨이 피드백 준 부분을 수정하면 더 좋을 것 같네요! 고생하셨습니다 👏🏻 🤓

@dev-Lena dev-Lena removed the Lena label May 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants