[WEAV-74] 프로필 입력 - 회사 선택 #29
Conversation
Walkthrough이번 변경 사항은 인증 회사 관련 기능을 추가하는 데 중점을 두고 있습니다. Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (24)
Projects/Core/Model/Sources/Network/CompanySearchResponse.swift (1)
11-19: 구현이 깔끔하고 적절합니다.
CompanySearchResponse구조체의 구현이 간결하고 명확합니다. 공개 접근 제어자를 사용하여 모듈 간 재사용성을 높인 점이 좋습니다.구조체와 프로퍼티에 대한 문서화 주석을 추가하면 더욱 좋을 것 같습니다. 예를 들어:
/// 회사 검색 응답을 나타내는 구조체 public struct CompanySearchResponse { /// 회사의 고유 식별자 public let id: String /// 회사의 이름 public let name: String /// 새로운 CompanySearchResponse 인스턴스를 생성합니다. /// - Parameters: /// - id: 회사의 고유 식별자 /// - name: 회사의 이름 public init(id: String, name: String) { self.id = id self.name = name } }이렇게 하면 API 문서 생성 시 더 자세한 정보를 제공할 수 있습니다.
Projects/Core/NetworkKit/Sources/CompanyService/CompanyService.swift (3)
13-16: 프로토콜 정의가 명확하며 개선이 필요한 부분이 있습니다.
CompanyServiceProtocol의 정의가 명확하고 Swift의 최신 기능을 잘 활용하고 있습니다. 다만, MARK 주석 형식을 개선할 수 있습니다.다음과 같이 MARK 주석 형식을 수정해주세요:
-//MARK: - Service Protocol +// MARK: - Service Protocol🧰 Tools
🪛 SwiftLint
[Warning] 13-13: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 13-13: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
18-22: 클래스 구현이 적절하며 작은 개선 사항이 있습니다.
CompanyService클래스가 싱글톤 패턴을 올바르게 구현하고 있으며,final로 선언된 것이 성능에 좋습니다. MARK 주석 형식만 개선하면 좋겠습니다.다음과 같이 MARK 주석 형식을 수정해주세요:
-//MARK: - Service +// MARK: - Service🧰 Tools
🪛 SwiftLint
[Warning] 18-18: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 18-18: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
24-37: 구현이 적절하며 오류 처리를 개선할 수 있습니다.
CompanyServiceProtocol의 구현이 올바르게 되어 있으며, async/await를 사용한 비동기 처리가 적절합니다. 다만, 오류 처리를 좀 더 강화할 수 있을 것 같습니다.다음과 같이 옵셔널 체이닝과 guard 문을 사용하여 더 안전한 구현을 할 수 있습니다:
public func requestSearchCompany(keyword: String) async throws -> [CompanySearchResponse] { let result = try await client.searchCompanies(query: .init(name: keyword)) guard let companies = result.ok?.body.json?.companies else { throw NSError(domain: "CompanyService", code: 0, userInfo: [NSLocalizedDescriptionKey: "Invalid response format"]) } return companies.map { CompanySearchResponse( id: $0.id, name: $0.name ) } }이렇게 하면 응답 데이터가 예상치 못한 형식일 경우에도 안전하게 처리할 수 있습니다.
Projects/Core/NetworkKit/Sources/CompanyService/CompanyServiceMock.swift (1)
12-12: SwiftLint 경고를 해결해 주세요.코드의 일관성과 Swift 스타일 가이드라인 준수를 위해 다음 SwiftLint 경고를 해결해 주시기 바랍니다:
- 주석 앞에 공백을 추가하세요.
- MARK 주석 형식을 수정하세요.
다음과 같이 수정하는 것을 제안합니다:
// MARK: - Service이렇게 수정하면 두 가지 경고를 모두 해결할 수 있습니다.
🧰 Tools
🪛 SwiftLint
[Warning] 12-12: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 12-12: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
Projects/DesignSystem/DesignCore/Sources/CTAButton/CTAButton.swift (1)
20-20: 이니셜라이저 변경 승인 및 개선 제안
titleColor매개변수를 이니셜라이저에 추가한 것은 적절합니다. 이는CTAButton의 사용성을 향상시키며, 기본값을.white로 설정하여 이전 버전과의 호환성도 유지하고 있습니다.개선 제안: 이 새로운 매개변수에 대한 문서화 주석을 추가하면 좋을 것 같습니다. 예를 들어:
/// 버튼 제목의 색상입니다. 기본값은 `.white`입니다. titleColor: Color = .white,이렇게 하면 API 사용자가 이 매개변수의 목적을 더 쉽게 이해할 수 있습니다.
Also applies to: 26-26
Projects/Features/SignUp/UnitTest/AuthCompanyTest.swift (3)
36-66: 회사 선택 흐름 테스트가 잘 구현되었습니다.
testCompanySelectionFlow메서드는 회사 선택 과정의 주요 시나리오를 잘 다루고 있습니다. 빈 검색어와 유효한 검색어에 대한 동작, 회사 선택 후 상태 변화, 그리고 새로운 검색어 입력 시 선택 및 검증 상태 초기화를 적절히 테스트하고 있습니다.다만, 비동기 작업 처리를 위해
Task.sleep을 사용하고 있는데, 이는 테스트의 안정성을 해칠 수 있습니다. 대신XCTestExpectation을 사용하여 비동기 작업의 완료를 기다리는 것이 더 좋은 방법일 수 있습니다. 예를 들어:let expectation = XCTestExpectation(description: "Company search completed") DispatchQueue.main.asyncAfter(deadline: .now() + 1.0) { expectation.fulfill() } wait(for: [expectation], timeout: 2.0)이렇게 하면 테스트의 안정성과 신뢰성을 높일 수 있습니다.
68-91: '내 회사가 없어요' 토글 기능 테스트가 잘 구현되었습니다.
testNotExistMyCompanyToggle메서드는 '내 회사가 없어요' 토글 기능의 주요 동작을 적절히 테스트하고 있습니다. 토글 후 상태 변화와 이전에 선택된 회사 정보가 지워지는지 확인하는 부분이 잘 구현되어 있습니다.개선을 위한 제안:
앞서 언급한 대로,
Task.sleep대신XCTestExpectation을 사용하여 비동기 작업을 처리하는 것이 좋습니다.테스트 케이스의 이름을 더 명확하게 할 수 있습니다. 예를 들어,
testNotExistMyCompanyToggleResetsSelectionAndDisablesInput과 같이 테스트의 목적을 더 자세히 설명할 수 있습니다.각 검증 단계에 대한 실패 메시지를 추가하면 테스트 실패 시 더 명확한 피드백을 얻을 수 있습니다. 예:
XCTAssertFalse(state.isTextFieldFocused, "텍스트 필드는 포커스를 잃어야 합니다.") XCTAssertFalse(state.isTextFieldEnabled, "텍스트 필드는 비활성화되어야 합니다.") XCTAssertTrue(state.isValidated, "상태는 여전히 유효해야 합니다.") XCTAssertNil(state.selectedCompany, "선택된 회사 정보는 초기화되어야 합니다.")이러한 개선사항들을 적용하면 테스트의 가독성과 유지보수성이 향상될 것입니다.
1-91: 전반적으로 잘 구현된 테스트 파일입니다.
AuthCompanyTest.swift파일은 AuthCompany 모듈에 대한 단위 테스트를 잘 구조화하여 구현하고 있습니다. 주요 시나리오를 커버하는 두 개의 테스트 메서드가 포함되어 있어 기능의 핵심 부분을 잘 검증하고 있습니다.전체적인 개선을 위한 제안:
비동기 작업 처리:
Task.sleep대신XCTestExpectation을 사용하여 테스트의 안정성을 높이세요.테스트 케이스 이름: 각 테스트 메서드의 이름을 더 구체적으로 지어 테스트의 목적을 명확히 하세요.
실패 메시지: 각 assertion에 실패 메시지를 추가하여 테스트 실패 시 더 명확한 피드백을 제공하세요.
엣지 케이스 테스트: 현재 테스트에서 다루지 않은 엣지 케이스(예: 네트워크 오류, 빈 검색 결과 등)에 대한 추가 테스트를 고려해보세요.
테스트 데이터 분리: 테스트에 사용되는 더미 데이터를 별도의 상수나 팩토리 메서드로 분리하여 테스트 코드의 가독성을 높이세요.
이러한 개선사항들을 적용하면 테스트 스위트의 품질과 유지보수성이 더욱 향상될 것입니다.
Projects/Core/CommonKit/Sources/Path/PathTypes.swift (3)
21-36: 디버그 프리뷰 타입이 적절히 확장되었습니다.
debugPreviewTypes배열에 새로운 케이스들이 추가되어 회원가입 프로세스의 모든 단계를 포함하고 있습니다. 이는 테스트와 디버깅에 유용할 것 같습니다.가독성을 위해 각 케이스에 간단한 주석을 추가하는 것을 고려해보세요. 예를 들어:
// 전화번호 입력 .signUp(.authPhoneInput), // 전화번호 인증 .signUp(.authPhoneVerify(...)), // 이용 약관 동의 .signUp(.authAgreement), // ... 기타 케이스들
69-69: 회원가입 하위 뷰 타입에 회사 인증이 추가되었습니다.
SignUpSubViewType열거형에.authCompany케이스를 추가한 것은 적절합니다. 이는PathType의 변경사항과 일관성이 있습니다.각 케이스에 관련된 데이터를 연관 값으로 추가하는 것을 고려해보세요. 예를 들어:
case authCompany(Company?)이렇게 하면 나중에 회사 정보를 쉽게 전달하고 관리할 수 있습니다.
90-93: 해시 함수가 적절히 업데이트되었습니다.
SignUpSubViewType.hash(into:)메서드에.authCompany케이스를 추가한 것은 적절합니다. 해시 값의 순서가 회원가입 프로세스의 순서를 반영하고 있는 것으로 보입니다.향후 유지보수를 위해 해시 값을 열거형의 원시값(raw value)으로 대체하는 것을 고려해보세요. 예를 들어:
public enum SignUpSubViewType: Int, Hashable { case authPhoneInput = 0 case authPhoneVerify = 1 // ... 기타 케이스들 case authCompany = 6 case authName = 7 }이렇게 하면
hash(into:)메서드를 별도로 구현할 필요가 없어지고, 새로운 케이스를 추가할 때 실수로 해시 값을 누락하는 것을 방지할 수 있습니다.Projects/DesignSystem/DesignCore/Sources/TextInput/TextInputView.swift (1)
28-28: LGTM:init메서드가 적절히 업데이트되었습니다.
isEnabled매개변수의 추가와 기본값 설정은 적절합니다. 이는 이전 버전과의 호환성을 유지하면서도 새로운 기능을 제공합니다.개선 제안:
isEnabled매개변수를leftView와rightIcon매개변수 앞으로 이동하는 것을 고려해 보세요. 이렇게 하면 관련 속성들이 더 가깝게 그룹화되어 가독성이 향상될 수 있습니다.public init( placeholder: String = "", backgroundColor: Color = .white, text: Binding<String>, keyboardType: UIKeyboardType = .default, isFocused: FocusState<Bool> = .init(), isEnabled: Bool = true, @ViewBuilder leftView: @escaping () -> Content = { EmptyView() }, rightIcon: TextInputRightIconModel? = nil )Also applies to: 39-39
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneInputView.swift (1)
52-72: 개발용 기능 추가에 대한 승인 및 제안개발 과정에서 전화번호 인증을 건너뛸 수 있는 기능을 추가한 것은 좋은 아이디어입니다. 다음 사항들을 고려해 보시기 바랍니다:
테스트 데이터의 유연성: 하드코딩된 값 대신 다양한 시나리오를 테스트할 수 있는 방법을 고려해 보세요. 예를 들어, 여러 테스트 계정을 선택할 수 있게 하는 것은 어떨까요?
문서화: 이 개발자 기능을 프로젝트의 개발 가이드라인에 문서화하여 팀원들이 인지하고 올바르게 사용할 수 있도록 하세요.
레이아웃 고려: 개발 빌드에서 이 버튼이 추가됨으로써 전체 레이아웃에 영향을 줄 수 있습니다. UI 테스트 시 이 점을 유의해야 합니다.
Projects/Core/NetworkKit/Sources/NetworkCore/Middleware/LogMiddleWare.swift (1)
80-82: 로깅 메서드의 개선 사항 승인
decodedPath변수를 도입하여 퍼센트 인코딩된 경로를 디코딩하는 방식으로 로깅 출력을 개선한 점이 좋습니다. 이는 로그의 가독성을 향상시킵니다.다음과 같은 작은 개선을 제안합니다:
let decodedPath = request.path?.removingPercentEncoding ?? request.path ?? "<nil>"이렇게 하면
removingPercentEncoding이 실패하더라도 원래의request.path를 사용할 수 있어, 더 많은 정보를 보존할 수 있습니다.Also applies to: 87-89
Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyIntent.swift (3)
16-16: 주석 형식 수정
//MARK: - Intent와 같은 주석은// MARK: - Intent로 변경하여//뒤에 공백을 추가하는 것이 좋습니다. SwiftLint에서는 주석 뒤에 최소 한 칸의 공백을 권장하며,// MARK: ...의 형식을 따르는 것이 좋습니다.Also applies to: 36-36, 55-55
🧰 Tools
🪛 SwiftLint
[Warning] 16-16: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 16-16: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
38-53: 프로토콜과 구조체의 위치 조정
Intentable프로토콜과DataModel구조체를extension내부에 정의하기보다는, 파일 혹은 클래스 외부에 정의하는 것이 코드의 가독성과 유지보수성에 더 좋습니다. 일반적으로 프로토콜과 구조체는 전역 또는 클래스 외부에서 정의됩니다.
76-77: 불필요한 빈 메서드 제거
task()메서드가 현재 빈 상태로 정의되어 있습니다. 만약 사용되지 않는다면 해당 메서드를 제거하는 것이 좋습니다.Projects/DesignSystem/DesignCore/Sources/DropDown/DropDownView.swift (1)
11-14: 'Equatable' 프로토콜 명시 제거 권장'Hashable' 프로토콜을 채택하면 자동으로 'Equatable' 프로토콜이 채택되므로, 별도로 'Equatable'을 명시할 필요가 없습니다.
다음과 같이 수정하는 것을 제안합니다:
-public protocol DropDownFetchable: Hashable, Equatable { +public protocol DropDownFetchable: Hashable {Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyModel.swift (4)
27-27:MARK주석의 형식을 표준에 맞게 수정해주세요
//MARK: Stateful주석은 표준 형식과 일치하지 않습니다. 슬래시 뒤에 공백을 추가하고,MARK:뒤에도 공백을 추가하여// MARK: - Stateful형식으로 변경해주세요.적용 가능한 변경 사항:
-//MARK: Stateful +// MARK: - Stateful🧰 Tools
🪛 SwiftLint
[Warning] 27-27: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 27-27: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
47-47:MARK주석의 형식을 표준에 맞게 수정해주세요
//MARK: State Properties주석도 표준 형식과 일치하지 않습니다. 동일한 방식으로 수정 부탁드립니다.적용 가능한 변경 사항:
-//MARK: State Properties +// MARK: - State Properties🧰 Tools
🪛 SwiftLint
[Warning] 47-47: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 47-47: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
75-75:MARK주석의 형식을 표준에 맞게 수정해주세요
//MARK: - Actionable주석에서 슬래시 뒤에 공백이 필요합니다. 표준 형식에 맞게 수정해주세요.적용 가능한 변경 사항:
-//MARK: - Actionable +// MARK: - Actionable🧰 Tools
🪛 SwiftLint
[Warning] 75-75: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 75-75: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
50-50: 옵셔널 변수의 불필요한 초기화를 제거해주세요옵셔널 변수
selectedCompany는 기본적으로nil로 초기화되므로= nil을 명시적으로 지정할 필요가 없습니다.적용 가능한 변경 사항:
-@Published var selectedCompany: CompanySearchResponse? = nil +@Published var selectedCompany: CompanySearchResponse?🧰 Tools
🪛 SwiftLint
[Warning] 50-50: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyView.swift (1)
146-152: [가독성 개선] 조건문 부정 연산자 사용 피하기코드에서 부정 연산자인
!를 사용하여 조건문을 작성하고 있습니다. 부정 연산자는 코드의 가독성을 떨어뜨릴 수 있으므로, 가능하다면 긍정 조건으로 재작성하는 것을 권장합니다.다음과 같이 조건문을 수정해 보세요:
if state.isNoCompanyHere { // 회사 정확하게 파악 불가하다면 다음 뷰로 intent.onTapNextButton() } else { // 회사를 정확하게 파악할 수 있다면 -> 같은 회사 매칭 팝업 보여주기 isShowSameCompanyPopup = true }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (17)
- Projects/App/Sources/Navigation/NavigationStack.swift (1 hunks)
- Projects/Core/CommonKit/Sources/Path/PathTypes.swift (4 hunks)
- Projects/Core/CoreKit/UnitTest/StringExtensionText.swift (1 hunks)
- Projects/Core/Model/Sources/Network/CompanySearchResponse.swift (1 hunks)
- Projects/Core/NetworkKit/Sources/CompanyService/CompanyService.swift (1 hunks)
- Projects/Core/NetworkKit/Sources/CompanyService/CompanyServiceMock.swift (1 hunks)
- Projects/Core/NetworkKit/Sources/NetworkCore/Middleware/LogMiddleWare.swift (1 hunks)
- Projects/DesignSystem/DesignCore/Sources/CTAButton/CTAButton.swift (1 hunks)
- Projects/DesignSystem/DesignCore/Sources/DropDown/DropDownView.swift (1 hunks)
- Projects/DesignSystem/DesignCore/Sources/TextInput/TextInputView.swift (5 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneInput/AuthPhoneInputView.swift (1 hunks)
- Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyView.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyIntent.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyModel.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyView.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthProfileAge/AuthProfileAgeInputIntent.swift (1 hunks)
- Projects/Features/SignUp/UnitTest/AuthCompanyTest.swift (1 hunks)
🧰 Additional context used
🪛 SwiftLint
Projects/Core/NetworkKit/Sources/CompanyService/CompanyService.swift
[Warning] 13-13: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 18-18: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 13-13: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 18-18: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
Projects/Core/NetworkKit/Sources/CompanyService/CompanyServiceMock.swift
[Warning] 12-12: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 12-12: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyIntent.swift
[Warning] 16-16: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 36-36: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 55-55: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 16-16: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 36-36: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 55-55: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyModel.swift
[Warning] 27-27: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 47-47: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 75-75: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 27-27: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 47-47: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 75-75: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 50-50: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
🔇 Additional comments (16)
Projects/Core/NetworkKit/Sources/CompanyService/CompanyService.swift (1)
1-11: LGTM: 파일 헤더와 임포트가 적절합니다.파일 헤더가 표준 형식을 따르고 있으며, 임포트된 모듈들이 회사 서비스 레이어에 적합해 보입니다.
Projects/Core/NetworkKit/Sources/CompanyService/CompanyServiceMock.swift (1)
1-11: 파일 구조와 import 문이 적절합니다.파일 구조가 잘 정리되어 있고, 필요한 모듈들이 적절히 import 되어 있습니다. 코드의 가독성과 유지보수성이 좋아 보입니다.
Projects/App/Sources/Navigation/NavigationStack.swift (1)
37-38: 회사 선택 뷰가 성공적으로 추가되었습니다.새로운
.authCompany케이스와AuthCompanyView()의 추가는 PR 목표와 일치하며, 기존 코드 구조를 잘 따르고 있습니다. 이는 사용자 등록 흐름에 회사 선택 기능을 원활하게 통합합니다.다음 스크립트를 실행하여
AuthCompanyView의 구현을 확인하겠습니다:Projects/DesignSystem/DesignCore/Sources/CTAButton/CTAButton.swift (2)
14-14: titleColor 프로퍼티 변경 승인
titleColor를 고정 값에서 변수로 변경한 것은 좋은 개선입니다. 이렇게 하면CTAButton의 유연성이 향상되어 다양한 디자인 요구 사항에 더 쉽게 대응할 수 있습니다.
Line range hint
1-58: 전체 변경 사항 요약 및 평가이번 변경으로
CTAButton의 유연성과 재사용성이 크게 향상되었습니다.titleColor를 커스터마이즈할 수 있게 됨으로써, 다양한 디자인 요구사항에 더 쉽게 대응할 수 있게 되었습니다.변경 사항들은 잘 구현되었으며, 기존 코드와 일관성을 유지하면서도 새로운 기능을 추가하였습니다. 또한, 기본값을 제공함으로써 이전 버전과의 호환성도 유지하고 있습니다.
전반적으로 이 PR은
CTAButton컴포넌트를 개선하는 데 성공적이었다고 판단됩니다. 문서화에 대한 작은 제안 외에는 추가적인 변경이 필요하지 않아 보입니다.Projects/Features/SignUp/Sources/ProfileInput/AuthProfileAge/AuthProfileAgeInputIntent.swift (1)
72-72: 변경 사항이 의도한 대로 구현되었습니다.이 변경으로 인해 사용자 흐름이
.authName에서.authCompany로 수정되었습니다. PR 목표와 일치하는 것으로 보입니다.다음 사항들을 확인해 주시기 바랍니다:
- 이 변경이 애플리케이션의 다른 부분과 일관성이 있는지 확인해 주세요.
- 사용자 흐름 문서나 관련 주석이 있다면 이 변경 사항을 반영하여 업데이트해 주세요.
다음 스크립트를 실행하여 애플리케이션의 다른 부분에서
.authName이나.authCompany를 사용하는 곳을 확인할 수 있습니다:이 결과를 바탕으로 필요한 경우 다른 파일들도 업데이트해 주세요.
✅ Verification successful
변경 사항이 성공적으로 검증되었습니다.
.authName에서.authCompany로의 변경이 애플리케이션의 다른 부분에서도 일관되게 반영되었음을 확인했습니다. 이로 인해 사용자 가입 흐름에 문제가 발생하지 않으며, 전체적인 동작이 의도한 대로 유지됩니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: 애플리케이션에서 .authName과 .authCompany 사용을 확인합니다. echo "Checking for .authName usage:" rg --type swift '.authName' echo "\nChecking for .authCompany usage:" rg --type swift '.authCompany'Length of output: 1305
Projects/Features/SignUp/UnitTest/AuthCompanyTest.swift (1)
15-34: 클래스 구조와 설정이 잘 구현되었습니다.
AuthCompanyTest클래스의 구조와 설정이 XCTest 프레임워크의 모범 사례를 잘 따르고 있습니다.setUp과tearDown메서드가 적절히 구현되어 있어 테스트 환경을 일관되게 유지하고 있습니다.Projects/Core/CommonKit/Sources/Path/PathTypes.swift (2)
54-54: 회사 인증 단계가 적절히 추가되었습니다.
PathType.name에.authCompany케이스를 추가한 것은 적절합니다. "내 직장 입력"이라는 이름은 해당 단계의 목적을 잘 설명하고 있습니다.
Line range hint
1-96: 전반적인 변경 사항이 PR 목표와 일치합니다.이 파일의 변경 사항은 회원가입 프로세스에 회사 선택 기능을 추가하는 PR의 목표를 잘 반영하고 있습니다.
PathType과SignUpSubViewType열거형에.authCompany케이스를 추가함으로써, 회사 인증 단계가 회원가입 흐름에 원활하게 통합되었습니다.주요 개선 사항:
- 디버그 프리뷰 타입에 새로운 회원가입 단계들이 추가되어 테스트와 개발이 용이해졌습니다.
- 회사 인증 관련 네비게이션 경로와 뷰 타입이 일관성 있게 추가되었습니다.
- 해시 함수가 적절히 업데이트되어 새로운 케이스를 지원합니다.
이러한 변경 사항들은 코드의 확장성과 유지보수성을 향상시키며, 향후 회원가입 프로세스의 추가 개선을 위한 견고한 기반을 제공합니다.
Projects/DesignSystem/DesignCore/Sources/TextInput/TextInputView.swift (2)
18-18: LGTM:isEnabled프로퍼티가 적절히 추가되었습니다.
isEnabled프로퍼티의 추가는 적절해 보입니다. 불변성을 위해let으로 선언되었고, 명시적인Bool타입 선언과 함께 Swift의 네이밍 컨벤션을 잘 따르고 있습니다.
Line range hint
1-93: 전반적인 변경사항에 대한 요약이번 PR에서
TextInputView.swift파일에 대한 변경사항은 전체적으로 잘 구현되었습니다.isEnabled속성의 추가로 텍스트 입력 필드의 활성화/비활성화 기능이 적절히 구현되었으며, 이전 버전과의 호환성도 유지되었습니다. 코드는 Swift의 모범 사례를 따르고 있으며 일관성이 있습니다.주요 개선 사항:
isEnabled속성 추가- 초기화 메서드 업데이트
- UI 상태 반영을 위한
body프로퍼티 수정제안된 개선사항:
init메서드에서isEnabled매개변수 위치 조정- 접근성 레이블 추가
이러한 변경사항은 사용자 경험을 향상시키고 코드의 유지보수성을 개선할 것으로 보입니다.
Projects/Features/SignUp/Sources/AuthSignUp/AuthPhoneVerify/AuthPhoneVerifyView.swift (1)
77-90:⚠️ Potential issue개발 환경에서의 인증 우회 기능 추가 검토
개발 및 스테이징 환경에서 전화 인증을 우회할 수 있는 기능이 추가되었습니다. 이는 개발 과정에서 유용할 수 있지만, 몇 가지 고려사항이 있습니다:
- 보안: 이 코드가 실제 프로덕션 빌드에 포함되지 않도록 주의해야 합니다.
- 사용자 경험: 개발자 모드임을 명확히 표시하여 실제 사용자와 구분해야 합니다.
- 코드 품질:
intent를 강제 캐스팅하는 것은 안전하지 않을 수 있습니다.다음과 같은 개선을 제안합니다:
- 프로덕션 빌드에서 이 코드가 제외되는지 확인하는 프로세스를 구현하세요.
- 개발자 모드임을 명확히 표시하는 UI 요소를 추가하세요.
intent캐스팅을 안전하게 처리하는 방법을 고려하세요. 예를 들어:if let authIntent = intent as? AuthPhoneVerifyIntent { let targetPath = authIntent.getNextPath(userType: .NEW) authIntent.pushNextView(to: targetPath) } else { print("Error: Unable to cast intent to AuthPhoneVerifyIntent") }다음 스크립트를 실행하여 프로덕션 빌드 설정을 확인하세요:
이 스크립트의 결과를 검토하여 프로덕션 코드에 디버그 기능이 포함되지 않았는지 확인하세요.
Projects/Core/CoreKit/UnitTest/StringExtensionText.swift (1)
46-49:⚠️ Potential issue전화번호 유효성 검사 로직 변경 확인 필요
전화번호에 하이픈이 포함된 경우의 유효성 검사 로직이 변경되었습니다. 이전에는 하이픈이 포함된 번호를 유효하지 않은 것으로 처리했지만, 이제는 유효한 것으로 간주합니다.
- 이 변경이 의도적이고 요구사항에 부합하는지 확인해 주세요.
- 변경된 로직을 반영하여
testInvalidPhoneNumber메서드 이름을testPhoneNumberValidation과 같이 더 일반적인 이름으로 변경하는 것이 좋겠습니다.isValidPhoneNumber()메서드의 구현이 이 변경사항을 반영하고 있는지 확인해 주세요.- 다른 관련 테스트 케이스들과 구현 코드를 검토하고 필요한 경우 업데이트해 주세요.
다음 스크립트를 실행하여
isValidPhoneNumber()메서드의 구현을 확인해 주세요:✅ Verification successful
전화번호 유효성 검사 로직 변경 확인 완료
isValidPhoneNumber()메서드는 하이픈을 제거한 후^010\d{8}$정규식을 사용하여 유효성을 검사하도록 구현되어 있습니다. 이로 인해 하이픈이 포함된 번호도 유효한 번호로 인식됩니다.변경 사항이 요구사항에 부합하며, 기존 테스트와 일관성이 있음을 확인했습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of isValidPhoneNumber() method # Test: Search for the isValidPhoneNumber() method implementation rg --type swift -A 10 'func isValidPhoneNumber\(\)'Length of output: 1047
Projects/Core/NetworkKit/Sources/NetworkCore/Middleware/LogMiddleWare.swift (2)
Line range hint
20-20:bodyLoggingPolicy의 가시성 변경 의도 확인 필요
bodyLoggingPolicy프로퍼티의 가시성이private에서package로 변경되었습니다. 이는 같은 패키지 내의 다른 컴포넌트에서 이 프로퍼티에 접근할 수 있게 합니다.이 변경의 의도와 잠재적인 영향을 확인해 주시기 바랍니다:
- 이 변경이 의도적인 것인지?
- 같은 패키지 내의 다른 컴포넌트에서 이 프로퍼티에 접근해야 할 필요성이 있는지?
- 이전에 캡슐화되어 있던 구현 세부사항이 노출될 가능성은 없는지?
이 변경이 필요하다면, 문서화를 통해 사용 목적과 주의사항을 명확히 설명하는 것이 좋을 것 같습니다.
Line range hint
22-25:LoggingMiddleware초기화 메서드의 가시성 변경 확인 필요
LoggingMiddleware의 초기화 메서드 가시성이private에서package로 변경되었습니다. 이는 같은 패키지 내의 다른 컴포넌트에서LoggingMiddleware인스턴스를 생성할 수 있게 합니다.이 변경의 의도와 잠재적인 영향을 확인해 주시기 바랍니다:
- 이 변경이 의도적인 것인지?
- 같은 패키지 내의 다른 컴포넌트에서
LoggingMiddleware인스턴스를 직접 생성해야 할 필요성이 있는지?- 이 변경이
bodyLoggingPolicy프로퍼티의 가시성 변경과 일관성 있게 적용된 것인지?이 변경이 필요하다면, 문서화를 통해 사용 목적과 주의사항을 명확히 설명하는 것이 좋을 것 같습니다.
Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyView.swift (1)
169-173: [확인 필요] 커스텀 모디파이어 및 AppCoordinator의 정의 확인
.textureBackground(),.setPopNavigation {},.setLoading(state.isLoading)등의 커스텀 모디파이어와AppCoordinator.shared.pop()이 사용되고 있습니다. 이러한 모디파이어와AppCoordinator가 프로젝트 내에서 올바르게 정의되어 있는지 확인해 주세요.다음 스크립트를 실행하여 해당 모디파이어와
AppCoordinator의 정의를 확인할 수 있습니다:#!/bin/bash # Description: 커스텀 모디파이어 및 AppCoordinator 정의 확인 # 커스텀 모디파이어의 정의 검색 rg --type swift 'func textureBackground\(' rg --type swift 'func setPopNavigation\(' rg --type swift 'func setLoading\(' # AppCoordinator의 정의 및 'shared' 인스턴스 검색 rg --type swift 'class AppCoordinator' rg --type swift 'static let shared' # AppCoordinator의 'pop' 메서드 정의 검색 rg --type swift 'func pop\('
| public final class CompanyServiceMock: CompanyServiceProtocol { | ||
|
|
||
| public init() {} | ||
|
|
||
| public func requestSearchCompany( | ||
| keyword: String | ||
| ) async throws -> [CompanySearchResponse] { | ||
| return [ | ||
| .init(id: "0", name: "현대글로비스"), | ||
| .init(id: "1", name: "현대자동차"), | ||
| .init(id: "2", name: "기아자동차"), | ||
| .init(id: "3", name: "채널톡"), | ||
| .init(id: "4", name: "닥터다이어리"), | ||
| .init(id: "5", name: "컬쳐커넥션"), | ||
| .init(id: "6", name: "엄청좋은회사"), | ||
| .init(id: "7", name: "로지텍"), | ||
| .init(id: "8", name: "스탠리"), | ||
| .init(id: "9", name: "스타벅스"), | ||
| .init(id: "10", name: "호날두주식회사"), | ||
| .init(id: "11", name: "애플"), | ||
| .init(id: "12", name: "엔비디아"), | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
mock 구현을 개선할 수 있는 몇 가지 제안사항이 있습니다.
현재 구현은 기본적인 기능을 제공하지만, 다음과 같은 개선사항을 고려해 보시기 바랍니다:
keyword매개변수를 활용하여 실제 검색을 시뮬레이션하는 것이 좋습니다. 현재는 이 매개변수가 사용되지 않고 있습니다.- 하드코딩된 mock 데이터 대신, 다양한 테스트 시나리오에 대응할 수 있는 유연한 데이터 생성 방식을 고려해 보세요.
- 네트워크 지연이나 잠재적 오류 상황을 시뮬레이션하는 기능을 추가하면 더욱 현실적인 테스트가 가능할 것 같습니다.
다음과 같은 개선된 구현을 제안합니다:
public final class CompanyServiceMock: CompanyServiceProtocol {
private let companies: [CompanySearchResponse]
private let delay: TimeInterval
private let shouldSimulateError: Bool
public init(companies: [CompanySearchResponse] = [], delay: TimeInterval = 0, shouldSimulateError: Bool = false) {
self.companies = companies
self.delay = delay
self.shouldSimulateError = shouldSimulateError
}
public func requestSearchCompany(keyword: String) async throws -> [CompanySearchResponse] {
if shouldSimulateError {
throw NSError(domain: "CompanyServiceMock", code: 0, userInfo: [NSLocalizedDescriptionKey: "Simulated network error"])
}
if delay > 0 {
try await Task.sleep(nanoseconds: UInt64(delay * 1_000_000_000))
}
return companies.filter { $0.name.lowercased().contains(keyword.lowercased()) }
}
}이 구현은 더 유연하고 현실적인 시나리오를 테스트할 수 있게 해줍니다. 필요하다면 이 구현을 적용하는 데 도움을 드릴 수 있습니다.
| @@ -44,7 +47,7 @@ public struct TextInput<Content>: View where Content: View { | |||
| DesignCore.Colors.grey100, | |||
| lineWidth: isFocused ? 1.0 : 0 | |||
| ) | |||
| .background(backgroundColor) | |||
| .background(isEnabled ? backgroundColor : DesignCore.Colors.grey100) | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
LGTM: body 프로퍼티의 변경사항이 적절합니다.
isEnabled 상태에 따른 배경색 변경과 TextField의 비활성화 처리가 잘 구현되었습니다. 이는 사용자에게 적절한 시각적 피드백을 제공합니다.
접근성 개선 제안: 비활성화된 상태일 때 시각적 피드백 외에도 접근성 레이블을 추가하는 것을 고려해보세요. 이는 스크린 리더 사용자에게 도움이 될 수 있습니다.
.accessibility(label: Text(isEnabled ? "활성화된 텍스트 입력" : "비활성화된 텍스트 입력"))이 수정사항을 TextField에 적용하면 접근성이 향상될 것입니다.
Also applies to: 63-63
| func searchCompanyData(keyword: String) async { | ||
| guard keyword.count > 0 else { | ||
| model?.setResponseData([]) | ||
| model?.setSelectedCompany(nil) | ||
| return | ||
| } | ||
| do { | ||
| let response = try await companyService.requestSearchCompany(keyword: keyword) | ||
| model?.setResponseData(response) | ||
| } catch { | ||
| print(error) | ||
| } | ||
| } |
There was a problem hiding this comment.
에러 처리 개선
searchCompanyData(keyword:) 메서드의 에러 처리에서 단순히 print(error)로 에러를 출력하고 있습니다. 사용자에게 적절한 에러 메시지를 표시하거나, 에러에 따른 처리 로직을 추가하는 것이 좋습니다. 또한 모델에 에러 상태를 전달하여 UI에서 에러를 반영할 수 있도록 하는 것을 고려해 보세요.
|
|
||
| public struct DropDownPicker<Content: View>: View { | ||
|
|
||
| @FocusState var showDropDown: Bool |
There was a problem hiding this comment.
'FocusState' 대신 'Binding' 사용 필요
'FocusState'는 주로 키보드 포커스 관리에 사용되며, 현재 상황에서는 'Binding'을 사용하는 것이 적절합니다. 외부에서 전달된 'FocusState'를 내부에서 프로퍼티 래퍼 변수에 할당하는 것은 컴파일 오류를 유발할 수 있습니다.
다음과 같이 수정하는 것을 제안합니다:
- @FocusState var showDropDown: Bool
+ @Binding var showDropDown: Bool
...
- showDropDown: FocusState<Bool>,
+ showDropDown: Binding<Bool>,
...
- self._showDropDown = showDropDown
+ self._showDropDown = showDropDown
...Also applies to: 52-52, 59-59
| RoundedRectangle(cornerRadius: 24) | ||
| .foregroundStyle(.white) // |
There was a problem hiding this comment.
'foregroundStyle(.white)' 대신 'fill(Color.white)' 사용 권장
'RoundedRectangle'에 색상을 적용할 때는 'foregroundStyle'보다 'fill'을 사용하는 것이 적절합니다.
다음과 같이 수정하는 것을 제안합니다:
RoundedRectangle(cornerRadius: 24)
- .foregroundStyle(.white)
+ .fill(Color.white)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RoundedRectangle(cornerRadius: 24) | |
| .foregroundStyle(.white) // | |
| RoundedRectangle(cornerRadius: 24) | |
| .fill(Color.white) |
| RoundedRectangle(cornerRadius: 24) | ||
|
|
||
| ) | ||
| .shadow(.default) |
There was a problem hiding this comment.
'shadow(.default)' 사용 오류 수정 필요
'shadow' 모디파이어에 '.default'를 사용하는 것은 컴파일 오류를 발생시킬 수 있습니다. 대신 적절한 형식의 'shadow'를 사용하세요.
다음과 같이 수정하는 것을 제안합니다:
- .shadow(.default)
+ .shadow(radius: 4)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .shadow(.default) | |
| .shadow(radius: 4) |
| DropDownPicker( | ||
| dataSources: state.searchResponse, | ||
| showDropDown: _showDropDown | ||
| ) { | ||
| TextInput( | ||
| placeholder: "내 회사 검색", | ||
| text: $container.model.textInput, | ||
| keyboardType: .namePhonePad, | ||
| isFocused: _showDropDown, | ||
| isEnabled: state.isTextFieldEnabled, | ||
| rightIcon: textInputRightIcon | ||
| ) | ||
| .interactiveDismissDisabled() | ||
| .textInputAutocapitalization(.never) | ||
| .autocorrectionDisabled() | ||
| .speechAnnouncementsQueued(false) | ||
| .speechSpellsOutCharacters(false) | ||
| } tapHandler: { index in | ||
| let company = state.searchResponse[index] | ||
| intent.onCompanySelected( | ||
| company: company | ||
| ) | ||
| } | ||
| .padding(.horizontal, 24) |
There was a problem hiding this comment.
[오류 수정] DropDownPicker의 클로저 구문 오류
DropDownPicker를 호출할 때 여러 개의 클로저 매개변수가 있을 경우, Swift에서는 명시적으로 매개변수 이름을 지정해야 합니다. 현재 코드에서는 두 번째 클로저 tapHandler가 후행 클로저로 잘못 사용되어 컴파일 에러가 발생할 수 있습니다.
다음과 같이 코드를 수정해 주세요:
DropDownPicker(
dataSources: state.searchResponse,
showDropDown: _showDropDown,
content: {
TextInput(
placeholder: "내 회사 검색",
text: $container.model.textInput,
keyboardType: .namePhonePad,
isFocused: _showDropDown,
isEnabled: state.isTextFieldEnabled,
rightIcon: textInputRightIcon
)
.interactiveDismissDisabled()
.textInputAutocapitalization(.never)
.autocorrectionDisabled()
.speechAnnouncementsQueued(false)
.speechSpellsOutCharacters(false)
},
tapHandler: { index in
let company = state.searchResponse[index]
intent.onCompanySelected(
company: company
)
}
)| public init() { | ||
| let model = AuthCompanyModel() | ||
| let intent = AuthCompanyIntent( | ||
| model: model, | ||
| input: .init() | ||
| ) | ||
| let container = MVIContainer( | ||
| intent: intent as AuthCompanyIntent.Intentable, | ||
| model: model as AuthCompanyModel.Stateful, | ||
| modelChangePublisher: model.objectWillChange | ||
| ) | ||
| self._container = StateObject(wrappedValue: container) | ||
| } |
There was a problem hiding this comment.
[주의] @StateObject 초기화 방식 검토
@StateObject 프로퍼티 래퍼인 container를 init 메서드에서 self._container를 사용하여 초기화하고 있습니다. SwiftUI에서는 @StateObject를 init에서 초기화할 경우 예상치 못한 동작이 발생할 수 있으므로, 선언 시 즉시 초기화하는 것을 권장합니다.
다음과 같이 수정하여 선언과 동시에 초기화해 보세요:
@StateObject private var container: MVIContainer<AuthCompanyIntent.Intentable, AuthCompanyModel.Stateful> = {
let model = AuthCompanyModel()
let intent = AuthCompanyIntent(
model: model,
input: .init()
)
return MVIContainer(
intent: intent as AuthCompanyIntent.Intentable,
model: model as AuthCompanyModel.Stateful,
modelChangePublisher: model.objectWillChange
)
}()| var bottomSpacingHeight: CGFloat { | ||
| return showDropDown ? Device.height * 0.7 : 0 | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
'Device.height'가 정의되어 있지 않습니다.
Device.height 정의를 찾을 수 없으며, 대신 UIScreen.main.bounds.height를 사용하는 것이 적절합니다.
🔗 Analysis chain
[확인 필요] 'Device.height'의 정의 확인
bottomSpacingHeight 계산에서 Device.height를 사용하고 있습니다. 프로젝트 내에 Device 구조체나 클래스가 정의되어 있는지 확인해 주세요. 만약 정의되어 있지 않다면, UIScreen.main.bounds.height를 사용하는 것이 일반적입니다.
다음 스크립트를 실행하여 Device.height의 정의 여부를 확인할 수 있습니다:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 'Device.height'의 정의 확인
# 'Device'의 정의 검색
rg --type swift --word-regex 'struct|class' -A 2 'Device'
# 'Device.height'의 사용 위치 검색
rg --type swift 'Device\.height'
Length of output: 3065
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/unitTest.yml (1)
17-19: 보안 관행 준수 및 개선 제안Secret.swift 파일 생성을 위해 GitHub 시크릿을 사용하는 것은 좋은 보안 관행입니다. 그러나 시크릿의 내용이 여러 줄일 경우 현재의 echo 명령어로는 제대로 처리되지 않을 수 있습니다.
다음과 같이 개선하는 것을 고려해 보세요:
- echo '${{ secrets.SECRET_SWIFT }}' > ./Projects/Core/CoreKit/Sources/Secret.swift + echo '${{ secrets.SECRET_SWIFT }}' | base64 --decode > ./Projects/Core/CoreKit/Sources/Secret.swift이 방식을 사용하려면 GitHub 시크릿을 설정할 때 내용을 base64로 인코딩해야 합니다. 이렇게 하면 여러 줄의 내용도 안전하게 처리할 수 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (4)
Projects/Core/NetworkKit/Sources/CompanyService/CompanyServiceMock.swift (1)
12-12: 코드 스타일과 포맷팅을 개선할 수 있습니다.
- MARK 주석 형식을 수정하세요. 올바른 형식은
// MARK: - ...입니다.- 주석 슬래시 뒤에 공백을 추가하세요.
수정된 버전:
// MARK: - Service이러한 작은 변경으로 코드의 일관성과 가독성이 향상됩니다.
🧰 Tools
🪛 SwiftLint
[Warning] 12-12: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 12-12: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyModel.swift (3)
25-80: MARK 주석 형식 개선 및 구조에 대한 피드백전반적인
AuthCompanyModel클래스와Stateful프로토콜의 구조가 잘 정의되어 있습니다. 상태 관리와 계산 속성의 구현이 명확하고 효과적입니다.다만, MARK 주석의 형식을 개선하여 코드의 가독성을 높일 수 있습니다. 다음과 같이 수정하는 것이 좋습니다:
// MARK: - Stateful // MARK: - State Properties이렇게 수정하면 Xcode의 심볼 네비게이터에서 더 잘 구분되어 코드 탐색이 용이해집니다.
🧰 Tools
🪛 SwiftLint
[Warning] 27-27: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 49-49: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 27-27: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 49-49: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 52-52: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 53-53: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
82-101: 프로토콜 설계 및 MARK 주석 개선
AuthCompanyModelActionable프로토콜의 설계가 매우 체계적이고 포괄적입니다. 콘텐츠 관리, 기본 상태 처리, 오류 처리 등 모델의 다양한 기능을 잘 정의하고 있습니다.MARK 주석의 형식을 다음과 같이 개선하면 더욱 좋겠습니다:
// MARK: - Actionable이렇게 수정하면 코드의 구조가 더욱 명확해지고, Xcode의 심볼 네비게이터에서 더 잘 구분됩니다.
🧰 Tools
🪛 SwiftLint
[Warning] 82-82: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 82-82: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
139-139: 불필요한 빈 메서드 제거 검토
setValidation(value:)메서드가 비어 있습니다. 이 메서드가 현재 필요하지 않다면 제거하는 것이 좋습니다. 향후 구현이 필요한 경우, 그때 추가하는 것이 코드의 명확성을 높일 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
- Projects/Core/NetworkKit/Sources/CompanyService/CompanyService.swift (1 hunks)
- Projects/Core/NetworkKit/Sources/CompanyService/CompanyServiceMock.swift (1 hunks)
- Projects/Core/NetworkKit/Sources/NetworkCore/Middleware/LogMiddleWare.swift (2 hunks)
- Projects/DesignSystem/DesignCore/Sources/DropDown/DropDownView.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyIntent.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyModel.swift (1 hunks)
- Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyView.swift (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyView.swift
🧰 Additional context used
🪛 SwiftLint
Projects/Core/NetworkKit/Sources/CompanyService/CompanyService.swift
[Warning] 13-13: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 21-21: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 13-13: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 21-21: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
Projects/Core/NetworkKit/Sources/CompanyService/CompanyServiceMock.swift
[Warning] 12-12: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 12-12: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyIntent.swift
[Warning] 16-16: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 36-36: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 59-59: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 16-16: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 36-36: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 59-59: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyModel.swift
[Warning] 27-27: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 49-49: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 82-82: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 27-27: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 49-49: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 82-82: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 52-52: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 53-53: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
🔇 Additional comments (8)
Projects/Core/NetworkKit/Sources/CompanyService/CompanyServiceMock.swift (2)
1-11: 파일 구조와 import 문이 적절합니다.파일 헤더와 import 문이 잘 구성되어 있습니다. Foundation과 Model을 import하여 필요한 기능을 사용할 수 있도록 준비되어 있습니다.
1-34: 이전 리뷰 의견을 반영하여 mock 구현을 개선해야 합니다.이전 리뷰에서 제안된 개선사항들이 아직 반영되지 않았습니다. 다음 사항들을 고려하여 구현을 개선해 주세요:
- 키워드를 사용한 실제 검색 시뮬레이션
- 다양한 테스트 시나리오에 대응할 수 있는 유연한 데이터 생성
- 네트워크 지연이나 오류 상황 시뮬레이션
이러한 개선사항들은 더 현실적이고 강력한 테스트를 가능하게 할 것입니다. 필요하다면 이러한 변경사항을 구현하는 데 도움을 드릴 수 있습니다.
🧰 Tools
🪛 SwiftLint
[Warning] 12-12: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 12-12: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyModel.swift (1)
1-157: 전체 코드 품질 및 구조에 대한 종합 평가
AuthCompanyModel.swift파일은 전반적으로 잘 구조화되어 있고, Swift의 특성과 프로토콜 지향 프로그래밍을 효과적으로 활용하고 있습니다. 주요 장점은 다음과 같습니다:
- 명확한 책임 분리:
Stateful프로토콜과AuthCompanyModelActionable프로토콜을 통해 모델의 상태와 행동을 잘 정의하고 있습니다.- 반응형 프로그래밍:
@Published속성을 적절히 사용하여 UI 업데이트를 용이하게 합니다.- 확장성: 프로토콜을 통한 설계로 향후 기능 확장이 용이합니다.
개선이 필요한 부분:
- MARK 주석 형식 통일
CompanySearchResponse의 해시 함수 최적화setToggleNoCompany()메서드의 복잡성 감소- 불필요한 빈 메서드 제거
이러한 minor한 이슈들을 해결하면 코드의 품질과 유지보수성이 더욱 향상될 것입니다.
🧰 Tools
🪛 SwiftLint
[Warning] 27-27: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 49-49: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 82-82: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 27-27: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 49-49: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 82-82: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 52-52: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 53-53: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
Projects/Core/NetworkKit/Sources/NetworkCore/Middleware/LogMiddleWare.swift (3)
85-86: 응답 로깅 방식 변경에 대한 승인요청 로깅 메서드와 일관된 변경사항으로, 승인됩니다. URL 디코딩과
이전 코멘트에서 언급한
Also applies to: 90-90
80-81: 로깅 방식 변경에 대한 승인 및 고려사항
logger에서decodedPath변수 도입은 승인됩니다. URL 디코딩은 로그의 가독성을 향상시킵니다.그러나
✅ Verification successful
로깅 방식 변경 승인
프로젝트 전반에 걸쳐
logger에서🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 프로젝트에서 사용 중인 다른 로깅 방식 확인 rg --type swift "(print|NSLog|os_log|Logger)" --glob '!**/LogMiddleWare.swift'Length of output: 1633
23-23: 로깅 용량 증가에 대한 승인 및 모니터링 제안기본 로깅 용량을 2048바이트에서 10000바이트로 증가시킨 변경사항은 승인됩니다. 이는 더 많은 요청 및 응답 본문을 로깅할 수 있게 해주어 디버깅에 도움이 될 것입니다.
그러나 이 변경이 성능과 메모리 사용에 미치는 영향을 모니터링하는 것이 좋습니다. 다음 스크립트를 실행하여 이 값이 다른 곳에서 사용되는지 확인해보세요:
✅ Verification successful
로깅 용량 증가에 대한 검증 완료
기본 로깅 용량을 2048바이트에서 10000바이트로 증가한 변경사항을 검증한 결과, 다른 파일에서 해당 값이나
bodyLoggingConfiguration이 사용되지 않는 것으로 확인되었습니다. 이에 따라 이 변경사항은 승인됩니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 다른 파일에서 2048 또는 10000 바이트 값이 사용되는지 확인 rg --type swift "(2048|10000).*bytes" --glob '!**/LogMiddleWare.swift'Length of output: 72
Script:
#!/bin/bash # LogMiddleWare.swift 외부에서 'bodyLoggingConfiguration'의 사용 여부 확인 rg --type swift 'bodyLoggingConfiguration\s*:\s*\w+' --glob '!**/LogMiddleWare.swift'Length of output: 87
Script:
#!/bin/bash # LogMiddleWare.swift 외부에서 'bodyLoggingConfiguration'이 사용된 초기화 메서드 찾기 ast-grep --lang swift --pattern $'init($_) { $$$ bodyLoggingConfiguration $_ $$$ }' --glob '!**/LogMiddleWare.swift'Length of output: 317
Script:
#!/bin/bash # LogMiddleWare.swift 외부에서 'bodyLoggingConfiguration'이 사용된 초기화 메서드 찾기 ast-grep --lang swift --pattern $'init($_) {\n $$$\n bodyLoggingConfiguration $_\n $$$\n}' -- --glob '!**/LogMiddleWare.swift'Length of output: 251
Projects/DesignSystem/DesignCore/Sources/DropDown/DropDownView.swift (1)
9-123: 구현이 적절하며 코드가 정확합니다
DropDownPicker의 전체적인 구현이 올바르며, 프로토콜과 뷰의 로직이 명확합니다.Projects/Features/SignUp/Sources/ProfileInput/AuthCompany/AuthCompanyIntent.swift (1)
81-81: 필요한 프로토콜 구현
task()메서드는 현재 빈 상태이지만,Intentable프로토콜 구현을 위해 필요한 것으로 보입니다. 이대로 두셔도 괜찮습니다.
| public final class CompanyServiceMock: CompanyServiceProtocol { | ||
|
|
||
| public init() {} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
초기화 메서드를 개선하여 더 유연한 테스트를 가능하게 할 수 있습니다.
현재 초기화 메서드는 매개변수가 없어 고정된 동작만 가능합니다. 다음과 같이 개선하여 다양한 테스트 시나리오를 지원할 수 있습니다:
public init(companies: [Model.CompanySearchResponse] = [], delay: TimeInterval = 0, shouldSimulateError: Bool = false) {
self.companies = companies
self.delay = delay
self.shouldSimulateError = shouldSimulateError
}이렇게 하면 테스트 시 다양한 상황을 시뮬레이션할 수 있습니다.
| public func requestSearchCompany(keyword: String, next: String?) async throws -> ([Model.CompanySearchResponse], String?) { | ||
| return ([ | ||
| .init(id: "0", name: "현대글로비스"), | ||
| .init(id: "1", name: "현대자동차"), | ||
| .init(id: "2", name: "기아자동차"), | ||
| .init(id: "3", name: "채널톡"), | ||
| .init(id: "4", name: "닥터다이어리"), | ||
| .init(id: "5", name: "컬쳐커넥션"), | ||
| .init(id: "6", name: "엄청좋은회사"), | ||
| .init(id: "7", name: "로지텍"), | ||
| .init(id: "8", name: "스탠리"), | ||
| .init(id: "9", name: "스타벅스"), | ||
| .init(id: "10", name: "호날두주식회사"), | ||
| .init(id: "11", name: "애플"), | ||
| .init(id: "12", name: "엔비디아"), | ||
| ], nil) | ||
| } |
There was a problem hiding this comment.
requestSearchCompany 메서드의 구현을 개선해야 합니다.
현재 구현은 실제 검색 동작을 정확히 시뮬레이션하지 않습니다. 다음과 같은 개선사항을 제안합니다:
keyword매개변수를 사용하여 회사 목록을 필터링하세요.next매개변수를 활용하여 페이지네이션을 시뮬레이션하세요.- 네트워크 지연이나 오류 상황을 시뮬레이션할 수 있도록 구현을 확장하세요.
예시 구현:
public func requestSearchCompany(keyword: String, next: String?) async throws -> ([Model.CompanySearchResponse], String?) {
if shouldSimulateError {
throw NSError(domain: "CompanyServiceMock", code: 0, userInfo: [NSLocalizedDescriptionKey: "Simulated network error"])
}
if delay > 0 {
try await Task.sleep(nanoseconds: UInt64(delay * 1_000_000_000))
}
let filteredCompanies = companies.filter { $0.name.lowercased().contains(keyword.lowercased()) }
let pageSize = 10
let startIndex = Int(next ?? "0") ?? 0
let endIndex = min(startIndex + pageSize, filteredCompanies.count)
let nextPage = endIndex < filteredCompanies.count ? String(endIndex) : nil
return (Array(filteredCompanies[startIndex..<endIndex]), nextPage)
}이 구현은 더 현실적인 검색 동작을 제공하며, 다양한 테스트 시나리오를 지원합니다.
| extension CompanySearchResponse: DropDownFetchable { | ||
| public static func == (lhs: CompanySearchResponse, rhs: CompanySearchResponse) -> Bool { | ||
| lhs.id == rhs.id | ||
| } | ||
|
|
||
| public func hash(into hasher: inout Hasher) { | ||
| hasher.combine(0) | ||
| } | ||
| } |
There was a problem hiding this comment.
해시 함수 구현 개선 필요
CompanySearchResponse의 hash(into:) 메서드가 여전히 모든 인스턴스에 대해 동일한 해시 값을 사용하고 있습니다. 이는 해시 기반 컬렉션의 성능을 저하시킬 수 있습니다.
다음과 같이 id를 사용하여 고유한 해시 값을 생성하도록 수정하는 것이 좋습니다:
public func hash(into hasher: inout Hasher) {
hasher.combine(id)
}이렇게 수정하면 각 CompanySearchResponse 인스턴스가 고유한 해시 값을 가지게 되어, 해시 테이블에서의 성능이 향상됩니다.
| func setToggleNoCompany() { | ||
| isNoCompanyHere.toggle() | ||
|
|
||
| // on 이 된다면 | ||
| if isNoCompanyHere { | ||
| selectedCompany = nil | ||
| textInput = "" | ||
| isTextFieldFocused = false | ||
| isTextFieldEnabled = false | ||
| } else { | ||
| isTextFieldEnabled = true | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
setToggleNoCompany() 메서드의 복잡성 검토
setToggleNoCompany() 메서드가 여러 상태를 동시에 변경하고 있습니다. 이는 side effect를 발생시킬 수 있고, 코드의 예측 가능성을 낮출 수 있습니다. 다음과 같은 개선을 고려해보세요:
- 메서드를 더 작은 단위의 메서드로 분리하여 각 동작의 책임을 명확히 합니다.
- 상태 변경의 이유와 영향을 주석으로 설명하여 다른 개발자들의 이해를 돕습니다.
- 가능하다면 상태 변경을 단일 책임 원칙에 따라 재구성해 보세요.
| import OpenapiGenerated | ||
| import Model | ||
|
|
||
| //MARK: - Service Protocol |
There was a problem hiding this comment.
// MARK: 주석 형식을 수정해야 합니다.
SwiftLint에 따르면, 주석('//') 뒤에 최소한 하나의 공백이 필요하며, 'MARK' 주석은 '// MARK: ...' 또는 '// MARK: - ...' 형식을 따라야 합니다.
다음과 같이 수정할 수 있습니다:
-//MARK: - Service Protocol
+// MARK: - Service Protocol
-//MARK: - Service
+// MARK: - ServiceAlso applies to: 21-21
🧰 Tools
🪛 SwiftLint
[Warning] 13-13: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 13-13: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
| } | ||
| } | ||
|
|
||
| //MARK: - Intentable |
There was a problem hiding this comment.
주석 형식 수정 필요
//MARK: - Intentable 주석은 표준 형식을 따르지 않습니다. 슬래시(//) 후에 공백을 추가하여 // MARK: - Intentable로 수정해주세요.
🧰 Tools
🪛 SwiftLint
[Warning] 36-36: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 36-36: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
| struct DataModel {} | ||
| } | ||
|
|
||
| //MARK: - Intentable |
There was a problem hiding this comment.
주석 형식 수정 필요
//MARK: - Intentable 주석은 표준 형식을 따르지 않습니다. 슬래시(//) 후에 공백을 추가하여 // MARK: - Intentable로 수정해주세요.
🧰 Tools
🪛 SwiftLint
[Warning] 59-59: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 59-59: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
| if let model = model as? AuthCompanyModel { | ||
| model.$textInput | ||
| .removeDuplicates() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
불필요한 다운캐스팅 제거 제안
onAppear 메서드에서 model을 AuthCompanyModel로 다운캐스팅하고 있습니다. model의 타입을 AuthCompanyModel로 지정하거나, AuthCompanyModelActionable 프로토콜에 필요한 프로퍼티와 메서드를 추가하여 다운캐스팅을 피할 수 있습니다.
| if let model = model as? AuthCompanyModel, | ||
| model.selectedCompany?.name != text { | ||
| model.setSelectedCompany(nil) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
불필요한 다운캐스팅 제거 제안
onTextChanged 메서드에서도 model을 AuthCompanyModel로 다운캐스팅하고 있습니다. 앞서 제안한 것처럼 model의 타입을 조정하거나 프로토콜을 확장하여 다운캐스팅을 제거할 수 있습니다.
| do { | ||
| let (response, next) = try await companyService.requestSearchCompany( | ||
| keyword: keyword, | ||
| next: next | ||
| ) | ||
| if needAppend { | ||
| model?.appendResponseData(response) | ||
| } else { | ||
| model?.setResponseData(response) | ||
| } | ||
| model?.setNextPaginationKey(next) | ||
| } catch { | ||
| print(error) | ||
| } |
There was a problem hiding this comment.
에러 처리 개선 필요
requestCompanyList 메서드에서 에러가 발생했을 때 단순히 print(error)로 출력하고 있습니다. 사용자에게 적절한 에러 메시지를 표시하거나 에러 상황에 대한 처리 로직을 추가하는 것이 좋습니다. 또한, 모델에 에러 상태를 전달하여 UI에서 에러를 반영할 수 있도록 고려해주세요.
구현사항
스크린샷(선택)
Summary by CodeRabbit
새로운 기능
버그 수정
테스트
문서화