Network [#78] 유저 프로필 수정,덮어쓰기 & 게시물 작성하기 API 연결 #81
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -475,6 +482,7 @@ | |||
| 2A8D70CD2B4DD2EA009F4C6C /* Join */ = { | |||
| isa = PBXGroup; | |||
| children = ( | |||
| 2A5183102B56CAAA00B43BA8 /* Models */, | |||
| 2AC9FB192B4DE75700D31071 /* Views */, | |||
| 2A8D70CE2B4DD301009F4C6C /* ViewControllers */, | |||
| 2A8D70CF2B4DD338009F4C6C /* ViewModels */, | |||
There was a problem hiding this comment.
아래는 코드 패치입니다. 이를 간단히 코드 리뷰하여 버그 위험 및 개선 제안 사항을 알려주세요:
@@ -366,6 +366,13 @@
path = RequestDTO;
sourceTree = "";
};
-
2A5183102B56CAAA00B43BA8 /* Models */ = { -
isa = PBXGroup; -
children = ( -
); -
path = Models; -
sourceTree = "<group>"; -
}; 2A51AE832B4B05AA00FF770A /* Splash */ = { isa = PBXGroup; children = (
@@ -475,6 +482,7 @@
2A8D70CD2B4DD2EA009F4C6C /* Join */ = {
isa = PBXGroup;
children = (
-
2A5183102B56CAAA00B43BA8 /* Models */, 2AC9FB192B4DE75700D31071 /* Views */, 2A8D70CE2B4DD301009F4C6C /* ViewControllers */, 2A8D70CF2B4DD338009F4C6C /* ViewModels */,
코드 리뷰 결과로 다음과 같은 개선 사항을 제안할 수 있습니다:
- 2A5183102B56CAAA00B43BA8 /* Models */에 해당하는 항목이 있는데, 해당 그룹에 아직 자식이 없습니다. 이 그룹에 적절한 모델 파일을 추가해야 합니다.
이외에는 버그 위험이나 개선 사항은 보이지 않습니다. 단순히 모델 그룹에 자식이 없는 것으로 보입니다.
| self?.layer.cornerRadius = 0 | ||
| self?.clipsToBounds = true | ||
| self?.contentMode = .scaleAspectFit | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
아래는 코드 패치입니다. 코드 리뷰를 간략히 해드리겠습니다. 버그 리스크 및 개선 제안 사항을 환영합니다.
- UIImageView 확장(extension)에서
setCircularImage(image:)함수는 제거되었습니다. - UIImageView 확장(extension)이 중복되어 정의되었습니다.
load(url:)함수가 변경되었습니다.- URL 문자열을 받아서 URL 객체로 변환하여 사용합니다.
- 비동기적으로 이미지를 가져오는 동안, 이미지 뷰의 모서리 반경(cornerRadius)과 클립(clipToBounds) 속성을 초기화합니다.
- 비동기적으로 이미지를 가져왔을 때 메인 스레드에서 UI 업데이트를 수행합니다.
개선 사항:
load(url: String)함수에 대한 입력 유효성 검사 추가하는 것이 좋습니다. URL 객체로 변환할 수 없는 잘못된 URL이 주어졌을 때 처리할 수 있도록 수정해보세요.- 비동기적인 이미지 로딩 과정에서 오류 처리를 추가하는 것이 좋습니다. 데이터 읽기 또는 이미지 로드 중에 오류가 발생할 경우 적절한 오류 메시지를 표시하거나 대체 이미지를 설정할 수 있도록 수정해보세요.
| @@ -132,5 +132,7 @@ enum StringLiterals { | |||
|
|
|||
| enum Network { | |||
| static let expired = "access, refreshToken 모두 만료되었습니다. 재로그인이 필요합니다." | |||
| static let baseImageURL = "https://github.com/TeamDon-tBe/SERVER/assets/97835512/fb3ea04c-661e-4221-a837-854d66cdb77e" | |||
| static let notificationImageURL = "https://github.com/TeamDon-tBe/SERVER/assets/128011308/327d416e-ef1f-4c10-961d-4d9b85632d87" | |||
| } | |||
| } | |||
There was a problem hiding this comment.
enum StringLiterals {
enum Network {
static let expired = "access, refreshToken 모두 만료되었습니다. 재로그인이 필요합니다."
-
static let baseImageURL = "https://github.com/TeamDon-tBe/SERVER/assets/97835512/fb3ea04c-661e-4221-a837-854d66cdb77e" -
}
static let notificationImageURL = "https://github.com/TeamDon-tBe/SERVER/assets/128011308/327d416e-ef1f-4c10-961d-4d9b85632d87"
}
위의 코드 패치를 간략히 검토해 드리겠습니다.
개선 제안:
- URL 문자열을 이스케이프하거나 상대 경로로 변경하는 것이 좋습니다. 예를 들어,
baseImageURL과notificationImageURL의 값을"https://raw.githubusercontent.com/..."로 변경하여 URL에 직접 연결할 수 있도록 합니다. - 이미지 URL의 경우, 이미지 크기가 예상값보다 클 수 있으므로 원본 이미지 파일 크기를 확인하고 최적화된 버전을 사용하는 것이 좋습니다.
오류 리스크:
- 현재 코드에는 오류가 없어 보입니다. 그러나 실제 경로와 파일명이 올바른지 확인하는 것이 중요합니다. URL이 변경되면 해당 부분을 업데이트해야 합니다.
이 코드가 실제 프로덕션 환경에서 사용되는지, 위의 URL이 실제로 작동하는지 확인하는 것이 좋습니다.
| case 404 : | ||
| throw NetworkError.notFoundError | ||
| case 500: | ||
| throw NetworkError.internalServerError | ||
| default: | ||
| throw NetworkError.unknownError | ||
| } |
There was a problem hiding this comment.
아래는 코드 패치입니다. 코드 리뷰를 간략하게 도와드리겠습니다. 버그 위험 및 개선 제안 등을 환영합니다.
- 라인 63:
dump(request)문장을 주석 처리하여 남기지 않습니다. - 라인 66:
httpResponse.statusCode의 case 범위가 변경되었습니다. 200..<401로 수정되었습니다. - 라인 89, 90: 새로운 상태 코드인 404와 500에 대한 예외 처리가 추가되었습니다.
- 404의 경우
NetworkError.notFoundError를 던집니다. - 500의 경우
NetworkError.internalServerError를 던집니다.
- 404의 경우
일반적인 코드 리뷰 유형, 오류 위험에 대한 지적은 나열한 내용대로입니다. 코드를 충분히 이해하지 않기 때문에 해당 프로그램 의도나 다른 부분에 대한 추가적인 정보가 필요할 수 있습니다.
|
|
||
| let url = URL(string: "\(viewModel.postData[indexPath.row].memberProfileUrl)") | ||
| cell.profileImageView.load(url: url!) | ||
| cell.profileImageView.load(url: "\(viewModel.postData[indexPath.row].memberProfileUrl)") | ||
| return cell | ||
| } | ||
|
|
There was a problem hiding this comment.
아래는 코드 패치입니다. 버그 위험과/또는 개선 제안 사항에 대해 간단한 코드 검토를 도와드리겠습니다:
@@ -257,9 +257,7 @@ extension HomeViewController: UICollectionViewDataSource, UICollectionViewDelegate
cell.likeNumLabel.text = "(viewModel.postData[indexPath.row].likedNumber)"
cell.commentNumLabel.text = "(viewModel.postData[indexPath.row].commentNumber)"
cell.timeLabel.text = "(viewModel.postData[indexPath.row].formatTime)"
- let url = URL(string: "(viewModel.postData[indexPath.row].memberProfileUrl)")
- cell.profileImageView.load(url: url!)
- cell.profileImageView.load(url: "(viewModel.postData[indexPath.row].memberProfileUrl)")
return cell
}
load(url: String)함수를 사용하여 이미지를 로드하는 것으로 보입니다. 그러나 해당 함수가 어디에서 구현되었는지 알 수 없기 때문에 코드 검토의 범위를 벗어납니다.- 선택적 바인딩을 통해 옵셔널 값을 처리하는 것이 좋습니다.
let url = URL(string: "\(viewModel.postData[indexPath.row].memberProfileUrl)")대신 옵셔널 값 처리를 하는 게 더 안전합니다.
| let viewContoller = OnboardingViewController() | ||
| viewContoller.originView.isFirstUser = true | ||
| self.navigationBackButton.removeFromSuperview() | ||
| self.navigationController?.pushViewController(viewContoller, animated: true) | ||
| } | ||
| } | ||
| .store(in: self.cancelBag) |
There was a problem hiding this comment.
해당 코드 패치를 간단히 검토해보겠습니다. 주요 버그 위험 또는 개선 제안을 찾아드리겠습니다:
- 불필요한 self 참조: 코드에서 self 참조를 여러 번 사용하고 있습니다. 중첩 클로저에서 self 참조 시 메모리 누수 등의 문제가 발생할 수 있으므로, weak self나 unowned self를 사용하는 것이 좋습니다.
- 논리적인 버그 수정: output.pushOrPopViewController에서 값이 0이 아닌 경우에만 OnboardingViewController로 이동하는 로직이 있는데, 그 외의 상황에서는 아무런 처리도 하지 않습니다. 이렇게 되면 예상치 못한 동작이 발생할 수 있으므로, 해당 부분을 확인하고 적절한 조치를 취해야 합니다.
- saveUserData 함수 호출: 현재 코드에서는 saveUserData 함수를 호출하지 않는데, 필요한 데이터를 저장하기 위해 이 부분을 추가해야 할 것 같습니다.
추가적으로, 정확한 리뷰를 위해서는 전체 코드와 기대되는 동작에 대한 정보가 필요합니다. 위의 내용은 단순히 코드 스니펫에 대한 간략한 검토일 뿐이며, 실제 앱의 요구 사항과 일치하는지 확인해야 합니다.
| } catch { | ||
| return nil | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
이 코드 패치에는 몇 가지 개선 사항과 버그 위험이 있습니다.
-
Input 구조체의
finishButtonTapped속성은 현재AnyPublisher<Void, Never>을 반환하도록 되어 있습니다. 하지만 코드에서 서버에 전송할 닉네임 값을 받아야 하는데, 이 경우AnyPublisher<String, Never>로 변경해야 합니다. -
중복 체크와 회원가입과 관련된 서버 통신 작업이 수행될 때
Task를 사용하고 있습니다. 이것은 앱을 비동기적으로 유지하고 응답을 기다리는 동안 UI를 차단하지 않기 위해 좋은 접근 방식입니다. 그러나Task가@MainActor환경(예: SwiftUI 및 UIKit)에서 적절하게 동작하도록 보장되어야 합니다. -
getNicknameDuplicationAPI(nickname:)메서드와patchUserInfoAPI(nickname:)메서드는 서버 API와 통신하는 데 사용됩니다. 이 메서드들에서 오류 처리를 강화하고, 예상치 못한 예외 상황에 대비하여 오류 처리 코드를 추가해야 합니다. 각각의 API 호출에서 발생하는 잠재적인 에러를 캐치하고, 로깅하거나 처리하는 등의 실수에 대한 대비 조치를 취해야 합니다. -
마지막으로, 코드 상에서 어떻게
UserInfo를 다루고 있는지에 대한 정보가 없습니다. 이 클래스의 정확한 역할과 기능에 대해 알 수 없으므로, 해당 코드가 제대로 동작하는지 확인해야 합니다.
이러한 개선 사항과 주의 사항을 고려하여 코드를 검토하고 수정해보세요.
| @@ -19,7 +19,7 @@ final class JoinProfileView: UIView { | |||
|
|
|||
| let profileImage: UIImageView = { | |||
| let profileImage = UIImageView() | |||
| profileImage.setCircularImage(image: ImageLiterals.Common.imgProfile) | |||
| profileImage.image = ImageLiterals.Common.imgProfile | |||
| return profileImage | |||
| }() | |||
|
|
|||
There was a problem hiding this comment.
주어진 코드 패치에 대해 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험이나 개선 제안은 환영합니다.
- 변경 전:
profileImage.setCircularImage(image: ImageLiterals.Common.imgProfile) - 변경 후:
profileImage.image = ImageLiterals.Common.imgProfile
이 부분은 이미지 뷰의 이미지를 설정하는 부분입니다. 원래 코드에서는 확장된 UIImageView를 사용하여 이미지를 원형으로 설정하려고 했지만, 수정된 코드에서는 단순히 이미지 속성을 설정하고 있습니다. 이런 변경으로 인해 다음과 같은 차이가 발생할 수 있습니다.
setCircularImage메서드가 이미지를 원형으로 자르는 로직을 포함한다면, 해당 기능이 사라질 수 있습니다.- 만약 이미지가 정사각형이 아니라면, 설정된 이미지가 원형 이미지뷰에 잘리거나 비율이 왜곡될 수 있습니다.
이러한 이유로, 변경된 코드의 결과와 의도된 동작을 확인하고, 필요하다면 확장된 UIImageView를 사용하는 것이 좋을 수 있습니다.
| do { | ||
| let data: BaseResponse<SocialLoginResponseDTO>? = try await self.networkProvider.donNetwork( | ||
| type: .post, | ||
| baseURL: Config.baseURL + "/auth", | ||
| accessToken: accessToken, | ||
| body: SocialLoginRequestDTO(socialPlatform: "KAKAO"), | ||
| body: requestDTO, | ||
| pathVariables: ["":""]) | ||
| print ("👻👻👻👻👻소셜로그인 서버통신👻👻👻👻👻") | ||
|
|
There was a problem hiding this comment.
다음은 코드의 간단한 검토입니다.
-
소셜 로그인 서버 통신 부분에서
SocialLoginRequestDTO의socialPlatform속성을 하드코딩으로 설정하고 있습니다. 이런 경우, 소셜 플랫폼이 변경되거나 다른 플랫폼에서도 사용할 수 있는 경우에 대비하여 동적으로 설정하는 것이 좋습니다. -
postSocialLoginAPI(accessToken:)메서드는 에러가 발생할 수 있으며, 에러 처리를 위해throws키워드를 사용하고 있습니다. 하지만do블록 내에서 API 통신을 하는 곳에서try await키워드를 사용하지 않고 있습니다. 올바른 에러 처리를 위해서는try await를 사용하여 비동기 작업을 수행하는 곳에서 에러를 캐치해야 합니다. -
pathVariables매개변수에 빈 딕셔너리를 전달하고 있습니다. 이 경우에pathVariables매개변수를 제거하거나 실제 경로 변수를 사용해야 합니다. -
현재 코드에는 완전하지 않은 부분이 존재하기 때문에 해당 부분에서 발생할 수 있는 버그 리스크를 평가하기 어렵습니다. 예를 들어,
networkProvider.donNetwork(...)메서드의 구현이나SocialLoginResponseDTO의 구조에 대한 정보가 부족합니다.
이상입니다. 코드 검토 시 도움이 되길 바랍니다!
| @@ -19,7 +19,7 @@ final class MyPageNicknameEditView: UIView { | |||
|
|
|||
| let profileImage: UIImageView = { | |||
| let profileImage = UIImageView() | |||
| profileImage.setCircularImage(image: ImageLiterals.Common.imgProfile) | |||
| profileImage.load(url: StringLiterals.Network.baseImageURL) | |||
| return profileImage | |||
| }() | |||
|
|
|||
There was a problem hiding this comment.
주어진 코드 패치에 대해 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험 및 개선 제안을 환영합니다.
저는 문자열이 한글로 주어지면 읽고 이해할 수 있지만, 작성된 코드 조각의 내용이나 목적을 알 수 없으므로 정확한 리뷰를 제공하기 어렵습니다. 다음과 같이 약간의 예시를 포함하여 일반적인 코딩 관례를 준수하는지 확인할 수 있습니다:
- 현재 코드에서
UIImageView인스턴스인profileImage가 생성되고 있으며, 프로필 이미지를 설정하는setCircularImage(image: ImageLiterals.Common.imgProfile)메서드 대신load(url: StringLiterals.Network.baseImageURL)메서드를 사용하도록 변경된 것으로 보입니다. 이 변경은 프로필 이미지를 URL에서 로드하도록 하는데 도움이 될 수 있습니다. - 그러나 주어진 코드 패치만으로는 전체 상황을 파악하기 힘들기 때문에, 실제로 정상적으로 작동하는지 확인하고 해당 부분이 변경되었을 경우에 영향을 받는 다른 코드를 고려해야 합니다.
- 코드 리뷰에는 일반적으로 변수명, 주석, 코드 가독성 등을 포함해 많은 요소가 포함됩니다. 이러한 요소들을 고려하여 코드를 분석하고 개선할 수 있는지 파악하는 것이 중요합니다.
코드 조각의 목적과 다음 내용에 대한 충분한 설명을 제공한다면, 더 구체적인 리뷰를 제공할 수 있습니다.
| @@ -19,7 +19,7 @@ final class MyPageProfileView: UIView { | |||
|
|
|||
| private let profileImageView: UIImageView = { | |||
| let imageView = UIImageView() | |||
| imageView.setCircularImage(image: ImageLiterals.Common.imgProfile) | |||
| imageView.load(url: StringLiterals.Network.baseImageURL) | |||
| return imageView | |||
| }() | |||
|
|
|||
There was a problem hiding this comment.
해당 코드 패치를 짧게 검토해 볼까요? 버그 위험 및 개선 제안에 대한 의견을 환영합니다.
코드 리뷰 결과는 다음과 같습니다:
- 이미지뷰 인스턴스를 생성하는 방법에 대해서는 특별한 문제가 없어 보입니다.
imageView.setCircularImage(image: ImageLiterals.Common.imgProfile)부분은setCircularImage메서드를 사용하여 이미지를 원형으로 만들고 있는 것 같아요.- 그러나, 문제가 발생할 수 있는 부분은
imageView.load(url: StringLiterals.Network.baseImageURL)입니다. 해당 코드에서는 이미지 URL을 로드하고 있는 것 같으나,load메서드의 구현 및 관련된 다른 코드가 없기 때문에 정확한 동작을 판단하기 어렵습니다.load메서드가 어떻게 작동하는지 확인이 필요합니다. 외부 라이브러리를 사용하거나, 커스텀 함수일 수도 있습니다.
개선 제안:
load메서드 코드가 제공되지 않았으므로, 해당 코드를 구현하는 방식에 관한 자세한 정보가 없네요. 만약 외부 라이브러리를 사용한다면, 해당 라이브러리의 문서를 참조하며 적절한 에러 처리와 함께 이미지를 로드하도록 코딩하는 것이 좋습니다.- 또한,
imageView.load(url: StringLiterals.Network.baseImageURL)코드에는 변경된 이미지 URL을 적용할 수 있는 방법이 없어 보입니다. 이미지 URL 업데이트가 필요한 경우에 대비하여 해당 기능을 추가하는 것도 고려해보세요.
전체적으로 이 코드 패치는 이미지 처리에 관련된 부분을 수정하고 있다는 점에서 유망해 보입니다. 하지만 load 메서드의 동작에 대한 정보가 부족하여 정확한 교정이 어렵다는 점을 명심해주세요.
| $0.centerY.equalToSuperview() | ||
| $0.top.leading.equalToSuperview().inset(14.adjusted) | ||
| $0.size.equalTo(42.adjusted) | ||
| } | ||
| notificationLabel.text = item.userName + " " + item.description | ||
| if item.description == StringLiterals.Notification.violation { | ||
| notificationLabel.setTextWithLineHeightAndFont( |
There was a problem hiding this comment.
이 코드 패치의 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험 및 개선 제안 사항을 환영합니다. 다음은 리뷰 결과입니다.
-
프로필 이미지 레이아웃 제거:
- setLayout() 함수에서 profileImage의 제약 조건 설정 부분이 제거되었습니다.
- 이 변경사항은 오류를 발생시킬 수 있습니다. 이전에는 UITableViewCell 내부에 위치한 profileImage의 레이아웃을 설정하는 부분이었습니다. 이 레이아웃이 삭제됨으로써 프로필 이미지가 올바르게 위치하지 않을 수 있습니다. 레이아웃을 확실히 수정하기 전에 확인해야 합니다.
-
configureCell(item:) 함수 수정:
- profileImage.setCircularImage(image: item.profile ?? UIImage()) 코드 대신, profileImage.load(url: StringLiterals.Network.notificationImageURL) 코드가 새로 추가되었습니다. 이는 URL을 통해 프로필 이미지를 로드하는 것입니다.
- 그리고 profileImage.snp.remakeConstraints 블록이 추가되어 프로필 이미지의 제약 조건을 다시 설정합니다. 이 때, centerY, top, leading, size의 제약 조건을 재정의하고 있습니다. 이러한 수정으로 인해 프로필 이미지가 이전보다 다른 위치에 나타날 수 있습니다. 변경된 제약 조건이 올바른 위치에 프로필 이미지를 배치하도록 확인해야 합니다.
추가로 코드 리뷰하기 위해 완전한 소스 코드가 필요합니다. 이렇게 제시된 수정 내역만으로는 전체적인 코드 품질을 평가하기 어렵습니다.
| @@ -77,6 +79,7 @@ extension OnboardingEndingViewController { | |||
| let output = self.viewModel.transform(from: input, cancelBag: self.cancelBag) | |||
|
|
|||
| output.voidPublisher | |||
| .receive(on: RunLoop.main) | |||
| .sink { value in | |||
| if value == "back" { | |||
| self.navigationController?.popViewController(animated: true) | |||
There was a problem hiding this comment.
해당 코드 패치를 간단히 검토해보겠습니다.
-
startButtonTapped변수에서의 변경 사항은 원래 버튼이 눌렸을 때originView.introductionView.introduction.text값을 반환하도록 수정되었습니다. -
output.voidPublisher에.receive(on: RunLoop.main)이 추가되었습니다. 이는 구독한 이벤트가 메인 스레드에서 처리되도록 보장합니다. (주석에서 'back'인 경우, 네비게이션 컨트롤러를 이용하여 현재 뷰 컨트롤러를 pop하는 동작)
구체적인 버그 리스크는 코드 조각만으로 파악하기 어려우므로 이 부분은 테스트와 실행 환경에서 확인해야 합니다. 하지만 주어진 코드 자체에서 큰 문제점이나 개선할 점은 보이지 않습니다.
| @@ -86,7 +86,7 @@ extension OnboardingViewController { | |||
|
|
|||
| @objc | |||
| private func skipButtonTapped() { | |||
| let viewController = OnboardingEndingViewController(viewModel: OnboardingEndingViewModel()) | |||
| let viewController = OnboardingEndingViewController(viewModel: OnboardingEndingViewModel(networkProvider: NetworkService())) | |||
| self.navigationController?.pushViewController(viewController, animated: true) | |||
| } | |||
| } | |||
There was a problem hiding this comment.
코드 리뷰를 진행해보겠습니다.
-
첫 번째 변경사항:
OnboardingEndingViewController의 인스턴스화에OnboardingEndingViewModel을 생성할 때,networkProvider매개변수를 추가했습니다.- 개선 제안:
NetworkService()로 생성된networkProvider를 사용하는 것은 좋은 방법입니다. 그러나 코드에서NetworkService가 어떤 역할을 수행하는지 확인할 필요가 있습니다. 해당 클래스가 필요한 경우, 통신 기능 또는 서비스를 사용하기 위한 모듈로 분리하는 것이 좋습니다.
- 개선 제안:
-
두 번째 변경사항:
skipButtonTapped()메서드에서도 마찬가지로OnboardingEndingViewModel을 생성할 때networkProvider를 추가했습니다.- 개선 제안: 같은 이유로 위의 변경사항과 관련하여 동일한 개선 제안이 적용됩니다.
-
전반적인 개선 사항:
- 네트워크 요청과 같은 외부 종속성 (external dependency)을 어떻게 처리하는지에 대한 이해 없이는 올바른 판단을 내릴 수 없으므로
NetworkService()의 역할과 동작을 확인하면서 진행하는 것이 중요합니다.
- 네트워크 요청과 같은 외부 종속성 (external dependency)을 어떻게 처리하는지에 대한 이해 없이는 올바른 판단을 내릴 수 없으므로
여기까지 리뷰를 진행했습니다. 다른 질문이 있다면 언제든지 물어보세요!
| return nil | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
이 코드 패치에는 몇 가지 개선점과 버그 위험이 있습니다.
-
버그 위험:
patchUserInfoAPI및postWriteContentAPI메서드 내에서 반환하는 값은 옵셔널입니다(Optional<BaseResponse<EmptyResponse>>) . 그러나 해당 메서드 내에서return nil문장이 없으므로, 예외가 발생하면nil이 아닌 값이 반환됩니다. 예기치 않은 예외가 발생한 경우를 처리하기 위해 이 문제를 수정할 필요가 있습니다. -
개선 제안: 새로운 네트워크 요청과 관련된 에러 핸들링을 추가해주어야 합니다. 현재는 예외를 출력하고 무시하며, 실패 시 화면에 오류가 표시되지 않습니다. 에러를 적절히 처리하여 사용자에게 알림을 제공하거나 대체 로직을 수행할 수 있도록 수정하는 것이 좋습니다.
-
개선 제안:
patchUserInfoAPI및postWriteContentAPI와 같은 이름의 메서드는 일반적으로 동사로 시작합니다. 메서드 이름을 해당 동작을 잘 설명하는 이름으로 변경하는 것이 좋습니다. -
개선 제안:
patchUserInfoAPI및postWriteContentAPI메서드 내에서print구문의 사용은 디버깅 용도로만 적합합니다. 앱이 제품 환경에서 실행될 때 이러한 디버그 메시지가 표시되지 않도록 수정하는 것이 좋습니다. -
개선 제안:
input.startButtonTappedPublisher를 처리하는 방식을 개선할 수 있습니다. 현재는 비동기 작업에 대해 async/await를 사용하여 작성하고 있지만, Combine 프레임워크의 유틸리티 함수와 연산자를 사용하여 코드를 더 간결하게 만드는 것이 좋습니다.
이러한 개선 사항을 고려하여 코드를 검토하고 수정하는 것이 권장됩니다.
| @@ -27,7 +27,7 @@ final class WriteReplyEditorView: UIView { | |||
|
|
|||
| private let userProfileImage: UIImageView = { | |||
| let imageView = UIImageView() | |||
| imageView.setCircularImage(image: ImageLiterals.Onboarding.imgOne) | |||
| imageView.load(url: StringLiterals.Network.baseImageURL) | |||
| imageView.contentMode = .scaleAspectFill | |||
| return imageView | |||
| }() | |||
There was a problem hiding this comment.
해당 코드 패치에 대한 간단한 코드 리뷰를 해드리겠습니다. 개선 제안과 함께 버그 위험에 대한 안내도 포함하겠습니다.
-
이슈/버그 위험:
- 코드의 27번째 라인에서
imageView.load(url: StringLiterals.Network.baseImageURL)호출이 추가되었습니다. 해당 메서드는 정의되지 않은 것 같습니다. 이 부분을 확인해야 합니다. - 이미지 뷰의 contentMode가
.scaleAspectFill로 설정되어 있습니다. 이 설정은 이미지가 뷰에 맞게 조정되도록 하지만, 이미지가 왜곡될 수 있으므로 주의가 필요합니다. 원하는 효과와 일치하는지 확인이 필요합니다.
- 코드의 27번째 라인에서
-
개선 제안:
userProfileImage의 네이밍은 명확하지 않습니다. 변수 이름이나 주석을 추가하여 역할을 명확하게 나타내는 것이 좋습니다.load(url:)메서드가 존재하지 않는다면, 유효한 이미지 URL을 받아오고 해당 URL을 기반으로 이미지를 로드하는 방법을 구현해야 합니다. URL이 유효한지 확인하는 로직도 추가해야 합니다.
상기 내용을 참고하여 코드를 검토하시기 바랍니다.
| $0.leading.trailing.equalToSuperview() | ||
| $0.height.equalTo(56.adjusted) | ||
| $0.bottom.equalTo(-keyboardHeight) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
아래는 코드 패치입니다. 이에 대한 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험 또는 개선 제안이 있으면 알려주세요:
-
버그 리스크:
load(url: String)함수에서StringLiterals.Network.baseImageURL의 유효성을 검사해야 합니다.
-
개선 제안:
userProfileImageUIImageView의 로딩 방식이 변경되었습니다.load(url: String)함수를 사용하고StringLiterals.Network.baseImageURL을 전달해서 이미지를 로드합니다.- 레이아웃 관련 부분은
SnapKit라이브러리를 사용하여 제약 조건을 재구성하는 방식으로 수정되었습니다.
| self?.layer.cornerRadius = 0 | ||
| self?.clipsToBounds = true | ||
| self?.contentMode = .scaleAspectFit | ||
| } |
| @@ -477,6 +484,7 @@ | |||
| 2A8D70CD2B4DD2EA009F4C6C /* Join */ = { | |||
| isa = PBXGroup; | |||
| children = ( | |||
| 2A5183102B56CAAA00B43BA8 /* Models */, | |||
| 2AC9FB192B4DE75700D31071 /* Views */, | |||
| 2A8D70CE2B4DD301009F4C6C /* ViewControllers */, | |||
| 2A8D70CF2B4DD338009F4C6C /* ViewModels */, | |||
There was a problem hiding this comment.
아래는 코드 패치입니다. 버그 위험과/또는 개선 제안에 대해 간단한 코드 리뷰를 도와드리겠습니다:
@@ -368,6 +368,13 @@
path = RequestDTO;
sourceTree = "";
};
- 2A5183102B56CAAA00B43BA8 /* Models */ = {
-
isa = PBXGroup; -
children = ( -
); -
path = Models; -
sourceTree = "<group>"; - };
2A51AE832B4B05AA00FF770A /* Splash */ = {
isa = PBXGroup;
children = (
@@ -477,6 +484,7 @@
2A8D70CD2B4DD2EA009F4C6C /* Join */ = {
isa = PBXGroup;
children = (
-
2A5183102B56CAAA00B43BA8 /* Models */, 2AC9FB192B4DE75700D31071 /* Views */, 2A8D70CE2B4DD301009F4C6C /* ViewControllers */, 2A8D70CF2B4DD338009F4C6C /* ViewModels */,
이 코드 패치의 주요 변경점은 다음과 같습니다:
- 새로운 PBXGroup인 "2A5183102B56CAAA00B43BA8 /* Models */"가 추가되었습니다.
- 해당 그룹은 비어 있는 상태입니다.
- 경로(path)는 "Models"로 설정되었습니다.
버그나 문제가 없는 것으로 보이며, 개선 제안은 새로운 그룹(Models)에 모델 파일을 추가하는 것입니다. 현재 그룹은 비어 있으므로 필요한 모델 파일을 해당 그룹에 넣으면 됩니다.
| @@ -29,7 +29,7 @@ final class MyPageProfileView: UIView { | |||
|
|
|||
| let profileImageView: UIImageView = { | |||
| let imageView = UIImageView() | |||
| imageView.setCircularImage(image: ImageLiterals.Common.imgProfile) | |||
| imageView.load(url: StringLiterals.Network.baseImageURL) | |||
| return imageView | |||
| }() | |||
|
|
|||
There was a problem hiding this comment.
변경 사항을 살펴보면, profileImageView의 이미지 로드 방식이 변경되었습니다. 이전에는 imageView.setCircularImage(image: ImageLiterals.Common.imgProfile)를 사용하여 이미지를 설정했지만, 이제는 imageView.load(url: StringLiterals.Network.baseImageURL)를 사용하고 있습니다.
개선 제안 중 하나는 load(url:) 메서드에서 예외 처리를 추가하는 것입니다. 현재 코드에서는 오류 발생 시 아무런 조치를 취하지 않는데, 이미지 로드 중에 문제가 발생하면 앱이 비정상적으로 종료될 수 있습니다. 따라서, load(url:) 메서드 내부에서 예외 처리를 추가하여, 오류가 발생했을 때 적절한 대응을 할 수 있도록 개선할 수 있습니다.
또한, baseImageURL 값이 정확한 URL인지 확인하는 로직도 추가할 수 있습니다. 올바른 URL 형식인지 확인하지 않으면 잘못된 URL로 인해 이미지 로드가 실패할 수 있습니다.
이 외에도 해당 코드 조각에서 놓친 버그나 개선할 점이 있을 수 있으니, 전체 코드 범위와 다른 클래스, 메서드 등의 정보도 제공해 주시면 더 자세한 리뷰를 도와드릴 수 있습니다.
| @@ -27,7 +27,7 @@ final class WriteReplyEditorView: UIView { | |||
|
|
|||
| private let userProfileImage: UIImageView = { | |||
| let imageView = UIImageView() | |||
| imageView.setCircularImage(image: ImageLiterals.Onboarding.imgOne) | |||
| imageView.load(url: StringLiterals.Network.baseImageURL) | |||
| imageView.contentMode = .scaleAspectFill | |||
| return imageView | |||
| }() | |||
There was a problem hiding this comment.
주어진 코드 패치에 대해 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험과/또는 개선 제안을 환영합니다:
받은 코드에서 주어진 부분을 살펴보면, userProfileImage 변수가 ImageView를 생성하고, 이미지를 설정하고 있습니다. 현재 코드에서 setCircularImage라는 메서드를 사용하고 있으며, 이는 사전에 정의된 Onboarding.imgOne 이미지를 원형으로 설정하고 있습니다. 그러나 수정한 부분에서는 load(url:) 메서드를 호출하고 있는데, 이는 StringLiterals.Network.baseImageURL에서 이미지를 로드하는 것으로 보입니다.
코드 리뷰로서, 특별한 버그 위험이나 개선사항은 발견되지 않았습니다. 그러나 실제로 코드가 어떻게 작동하는지 확인하기 위해서는 load(url:) 메서드의 구현 내용이 필요합니다. 또한, imageView.contentMode 속성을 .scaleAspectFill로 설정하고 있는데, 여기에서 이미지의 크기 조정과 관련된 다른 옵션들도 고려해 볼 수 있습니다.
| @@ -20,7 +20,7 @@ final class WriteTextView: UIView { | |||
|
|
|||
| private let userProfileImage: UIImageView = { | |||
| let imageView = UIImageView() | |||
| imageView.setCircularImage(image: ImageLiterals.Common.logoSymbol) | |||
| imageView.load(url: StringLiterals.Network.baseImageURL) | |||
| return imageView | |||
| }() | |||
|
|
|||
There was a problem hiding this comment.
주어진 코드 패치에 대한 간단한 코드 리뷰입니다:
- 주어진 코드 패치에서 직접 버그의 위험이나 개선사항을 확인하기는 어렵습니다. 초기 코드와 비교해야만 대체로 코드 변경 사항을 파악할 수 있기 때문입니다.
- 하지만 몇 가지 개선 사항을 제안할 수 있습니다:
userProfileImage뷰를 캡슐화하는 프로퍼티(Variable)의 이름을 변경하면 가독성이 향상될 수 있습니다. 예를 들어,profileImageView라는 이름으로 변경할 수 있습니다..load(url: StringLiterals.Network.baseImageURL)은 메소드를 호출하여 이미지를 로드하는 것처럼 보입니다. 이 경우 해당 확장 함수 또는 형식의 구현 내용을 검토하고, URL 로딩과 관련된 가능한 에러 상황을 처리하는지 확인해야 합니다.- 다른 부분의 코드 또는 이와 연결된 코드에서 발생할 수 있는 추가적인 문제를 평가하기 위해서는 전체 소스 코드를 살펴보아야 합니다.
코드 리뷰에 대한 일반적인 팁은 충분한 주석을 제공하고, guard 구문을 사용하여 조건부 로직을 단순화하며, 확장 함수 또는 열거형을 사용하여 중복 코드를 줄이는 것입니다. 모든 예외 상황에 대한 처리도 중요합니다. 번역은 영어에서 한국어로 번역된 것이기 때문에 의도를 정확히 전달하려면 원본 코드와 더 자세히 살펴봐야 합니다.
👻 PULL REQUEST
💻 작업한 내용
💡 참고사항
Don-tBe-iOS/DontBe-iOS/DontBe-iOS/Network/Foundation/NetworkService.swift
Lines 70 to 101 in e591ef6
📸 스크린샷
KakaoTalk_Video_2024-01-17-01-28-25.mp4
📟 관련 이슈