[Feat-T3-60] Persistence 구현#8
Conversation
| import Foundation | ||
|
|
||
| /// 간단한 Key-Value 저장을 위한 UserDefaults Store | ||
| public final class SimpleUserDefaultsStore { |
There was a problem hiding this comment.
KeyChain은 KeychainStorage이고, UserDefaults는 SimpleUserDefaultsStore인데
둘이 통일 없이 왜 Storage, Store 로 나눈 이유가 궁금합니다.
또한 UserDefaults 앞에 Simple을 붙인 이유가 따로 있으실까요 ?
There was a problem hiding this comment.
KeychainStorage는 Keychain 접근 자체를 래핑한 낮은 수준의 추상화 역할을 하기 때문에, 내부적으로 데이터를 저장/조회하는 "저장소" 역할이라는 의미에서 Storage로 네이밍했습니다.
반면, UserDefaults 관련 클래스는 실제 값을 저장/조회하는 응용 계층 수준에서 사용될 수 있도록 설계했고, 이는 Repository나 UseCase에서 직접 사용하는 구조이기 때문에 Store로 네이밍했습니다.
SimpleUserDefaultsStore라는 이름은 Generic하게 사용할 수 있는 Key-Value 저장소라는 점을 명확히 하기 위해 붙인 네이밍입니다.
There was a problem hiding this comment.
PR Description에는 "UserDefaultsStorage / KeychainStorage를 공통 프로토콜 기반으로 통일" 이라고 기재되어 있었습니다.
작업 내용과 작성된 PR 내용이 일치하지 않아 혼돈이 오는 것 같습니다.
원활한 협업을 위해 더 신경써서 작성해주시면 좋을 것 같아요.
SimpleUserDefaultsStore라는 이름은 Generic하게 사용할 수 있는 Key-Value 저장소라는 점을 명확히 하기 위해 붙인 네이밍입니다.
라고 하셨는데 UserDefaults를 Key-Value로 저장하는 것외에 다른 방식으로 저장하는 것이 있나요 ? 있다면 그것을 이번 프로젝트에서 사용하실건가요 ??
앞에 Simple이 붙는 것은 다른 용도의 UserDefaults도 있다는 것으로 생각이 들 수 있을 것 같은 네이밍입니다.
하나의 UserDefaults만 사용할 것이라면 Simple을 제거하는 것이 코드를 이해하는데 도움이 될 것 같습니다.
또한 UserDefaults를 UseCase에서 직접 사용하는 구조라고 하셨는데, Domain이 DataSource를 거쳐 Persistence에 접근하지 않나요 ? 어떻게 UseCase에서 UserDefaults를 직접 사용하는 구조인지 궁금합니다.
There was a problem hiding this comment.
현재 구조는 UseCase가 UserDefaults를 직접 사용하는 것이 아니라 UserDefaultsStorage를 간접적으로 사용하고 있습니다.실제 의존성을 의미한 것이 아니라 UseCase의 주요 흐름 안에서 사용되는 저장소라는 의미였습니다
generalban
force-pushed
the
feat/persistence-t3-60
branch
from
c9462ec to
0318255
Compare
choijungp
left a comment
choijungp
left a comment
There was a problem hiding this comment.
혹시 push 하기 전에 build 하셨나요 ? 현재 브랜치 기준으로 빌드하면 에러가 발생합니다.
KeychainStorage이 KeyValueStorageProtocol를 채택하고 있는데 구현 내용이 달라 발생하는 에러입니다.
앞으로 푸시하기 전에 빌드 후 에러가 없는지 확인해주세요.
generalban
force-pushed
the
feat/persistence-t3-60
branch
from
dee83b1 to
e2c06ce
Compare
| import Foundation | ||
|
|
||
| /// 간단한 Key-Value 저장을 위한 UserDefaults Store | ||
| public final class UserDefaultsStorage { |
There was a problem hiding this comment.
UserDefaultsStorage가 채택할 Protocol이 구현되어 있지 않습니다.
DataSource에 UserDefaultsStorageProtocol 구현 후 채택해주세요.
generalban
force-pushed
the
feat/persistence-t3-60
branch
from
51025a1 to
4a6c08a
Compare
generalban
force-pushed
the
feat/persistence-t3-60
branch
from
3ae9c45 to
73b5f2e
Compare
| public func save(value: Any?, forKey key: String) { | ||
| userDefaults.set(value, forKey: key) | ||
| } | ||
|
|
||
| public func load<T>(forKey key: String) -> T? { | ||
| userDefaults.object(forKey: key) as? T | ||
| } | ||
|
|
||
| public func remove(forKey key: String) { | ||
| userDefaults.removeObject(forKey: key) | ||
| } |
There was a problem hiding this comment.
UserDefaultsStorageProtocol 에 정의되어 있는것도 채택해서 구현하는 타입이 달라 에러납니다.
public protocol UserDefaultsStorageProtocol {
func save(_ value: String?, forKey key: String) -> Bool
func load(forKey key: String) -> String?
func remove(forKey key: String) -> Bool
}
그럼 당연히 에러가 발생하지 않을까요 ? 푸시 전에 직접 빌드해서, 에러 없는지 확인하고 작업해주세요.
There was a problem hiding this comment.
리턴값이 있는 함수의 결과를 무시하면 경고를 띄워서 이걸 피하려고 @discardableResult를 붙여줬습니다
generalban
force-pushed
the
feat/persistence-t3-60
branch
from
74e92ae to
7061e6b
Compare
| public func save(_ value: String?, forKey key: String) -> Bool { | ||
| userDefaults.set(value, forKey: key) | ||
| return (userDefaults.string(forKey: key) == value) | ||
| } |
There was a problem hiding this comment.
제가 봤을 때 protocol을 수정하는게 맞을 것 같아요. UserDefaults에 String 값만 저장할건가요 ?
왜 String? 타입만을 저장하는지 모르겠습니다.
There was a problem hiding this comment.
혹시 몰라 말씀드리는데 save, load 둘 다 말씀드린겁니다.
There was a problem hiding this comment.
Any?, T? 기반으로 유연하게 가는 방향으로 할게요
There was a problem hiding this comment.
save 함수도 근데 옵셔널로 가는 이유는 무엇일까요? save 할 때는 항상 Value 값이 존재하는거 아닌가요?
generalban
force-pushed
the
feat/persistence-t3-60
branch
from
2f08823 to
c2ab2bc
Compare
2 participants
🌁 Background
Persistence 모듈이 Domain을 import하고, 특정 구조체에 의존하는 문제가 있었습니다.
흐름을 정리하고, KeyValueStorage 기반으로 추상화하여 중복 구현을 제거했습니다.
👩💻 Contents
KeyValueStorable제네릭 구조로 저장소 역할을 분리✅ Testing
📝 Review Note
📣 Related Issue
📬 Reference