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

KeyChainWrapper 추가 #8

Merged
merged 4 commits into from
Mar 14, 2025
Merged

KeyChainWrapper 추가 #8

merged 4 commits into from
Mar 14, 2025

Conversation

ji-yeon224
Copy link
Collaborator

#️⃣ Issue Number

🔎 작업 내용

키체인 CRUD 기능 추가하였습니다.

📷 스크린샷

🛠️ 기타 공유 내용

  • EntryPoint를 설정하고, 빌드 확인을 위해 임시로 ContentView() 를 추가했습니다.
  • 빌드 오류가 나는 부분 주석 처리 및 수정하였습니다.

@ji-yeon224 ji-yeon224 requested a review from usa4060 March 13, 2025 11:35
@ji-yeon224 ji-yeon224 self-assigned this Mar 13, 2025
@ji-yeon224 ji-yeon224 linked an issue Mar 13, 2025 that may be closed by this pull request
1 task
@usa4060
Copy link
Contributor

usa4060 commented Mar 13, 2025

고생하셨습니다~
한 가지 생각해보면 좋을 부분이 있는데, KeychainWrapperstruct로 구현하는건 어떨까요?
현재 내부의 save, read, update, delete 등의 간단한 함수만 존재하는 상태인데, static func로 처리하면 KeychainWrapper.save("abcd1234", forKey: .accessToken) 이런식으로 필요할 때만 따로 호출하면서 메모리를 덜 사용할 수 있을 것 같다는 생각임다


/// 키체인 키 설정
public enum KeychainKey: String {
case accessToken
Copy link
Contributor

Choose a reason for hiding this comment

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

KeychainKey는 따로 파일로 빼서 관리하는게 보기 편할 것 같아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

726e2a8
반영했습니다~

@ji-yeon224
Copy link
Collaborator Author

고생하셨습니다~ 한 가지 생각해보면 좋을 부분이 있는데, KeychainWrapperstruct로 구현하는건 어떨까요? 현재 내부의 save, read, update, delete 등의 간단한 함수만 존재하는 상태인데, static func로 처리하면 KeychainWrapper.save("abcd1234", forKey: .accessToken) 이런식으로 필요할 때만 따로 호출하면서 메모리를 덜 사용할 수 있을 것 같다는 생각임다

피드백 감삼다!
코멘트대로 Struct로 변경하는 것이 더 효율적일 것이라 생각해서 변경하였습니다!
d5f8f4d

@ji-yeon224 ji-yeon224 requested a review from usa4060 March 13, 2025 16:28
Copy link
Contributor

@usa4060 usa4060 left a comment

Choose a reason for hiding this comment

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

확인했습니다~!

@ji-yeon224 ji-yeon224 merged commit 681b14b into develop Mar 14, 2025
@ji-yeon224 ji-yeon224 deleted the feature/keychain branch March 15, 2025 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] KeyChain 추가
2 participants