-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feat [#31] 회원가입_프로필 UI 구현 #32
Conversation
3CE9C1322B4C3C2D0086E4A3 /* UITextView+.swift in Sources */, | ||
2A8D70BF2B4D68E3009F4C6C /* OnboardingEndingViewController.swift in Sources */, | ||
2A31FF572B4F1E0400FEEED9 /* String+.swift in Sources */, | ||
2A51AE852B4B05AA00FF770A /* SplashViewController.swift in Sources */, | ||
2A8D70C32B4D7FF5009F4C6C /* BackButton.swift in Sources */, | ||
2A8D70D32B4DD360009F4C6C /* JoinAgreeViewModel.swift in Sources */, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an AI language model, I don't have real-time information or the ability to predict the future beyond my last update in September 2021. Therefore, I cannot provide specific information about the events of 2023 or 2024. Please refer to reliable news sources or other up-to-date references for the latest information.
|
||
return consonantScalarRange ~= scalar | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰:
- 코드의 목적이나 기능 설명이 없습니다. 주석을 통해 코드의 목적과 역할을 명확하게 설명하는 것이 좋습니다.
String+.swift
파일의 이름에서 알 수 있듯이, 해당 코드는String
타입에 대한 확장(extension)입니다.isConsonant
프로퍼티를 추가하여 입력된 문자열이 자음인지 여부를 판단하는 기능을 제공합니다.- 문자열을 유니코드 스칼라로 변환하여 값이 존재하면 해당 스칼라가 일정 범위 내에 있는지 확인합니다.
consonantScalarRange
상수는 자음 스칼라 값(한글 자음의 유니코드 값 범위)을 나타내며, 해당 범위 안에 스칼라 값이 존재하는지 체크합니다.- 가독성과 명확성을 위해서, 변수나 함수에 설명적인 이름을 사용하는 것이 좋습니다.
버그 위험점:
guard
문에서 문자열을 유니코드 스칼라로 변환할 때, 옵셔널 값이 아닌UnicodeScalar(self)
로 직접 변환하는데, 이때self
가 빈 문자열일 경우nil
이 반환되고guard
조건문을 통과하게 됩니다. 빈 문자열에 대한 처리를 추가하는 것이 좋습니다.
개선 제안:
- 주석을 추가하여 코드의 목적과 역할을 명확하게 설명합니다.
consonantScalarRange
상수를 표현적인 이름으로 변경하면 가독성이 개선됩니다.- 반환 타입인
Bool
대신에isConsonant
의 로직이 복잡해지거나 다른 타입으로 확장될 가능성이 있는 경우, 구조체(ConsonantChecker
등)로 추상화하여 좀 더 유연한 구조를 만들 수 있습니다. - 옵셔널 바인딩(optional binding)을 사용하여
self
가 빈 문자열인 경우 조건문에서 거부되도록 변경합니다.
@@ -52,6 +52,7 @@ enum ImageLiterals { | |||
static var btnNecessary: UIImage { .load(name: "btn_necessary") } | |||
static var btnSelect: UIImage { .load(name: "btn_select") } | |||
static var btnView: UIImage { .load(name: "btn_view") } | |||
static var btnPlus: UIImage { .load(name: "btn_plus") } | |||
} | |||
|
|||
enum Home { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 패치는 이미지 리터럴을 정의하는 열거형에 변경 사항을 추가합니다.
변경 사항:
Common
열거형에imgProfile
이미지 리터럴이 추가되었습니다.TabBar
열거형에서imgProfile
이미지 리터럴을 제거하였습니다.Login
열거형에btnPlus
이미지 리터럴이 추가되었습니다.
버그 위험사항:
본 코드 패치에서는 특정한 버그 위험이나 오류를 발견하지 못했습니다.
개선 제안:
- 이미지 리터럴 이름을 좀 더 명확하게 나타낼 수 있는 방법과 일관성을 유지하는 것을 고려해 볼 수 있습니다.
- 이미지 파일의 이름을 일관된 형식으로 관리하여 코드에서 사용하는 이름과 일치시킵니다.
"author" : "xcode", | ||
"version" : 1 | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 패치는 JSON 포맷으로 작성된 이미지 정보를 포함하고 있습니다. 이미지 파일 이름과 해당 이미지의 idiomatic 속성(일반적인 사용법), scale 속성(해상도 배율) 등의 정보가 포함되어 있습니다. 버전 및 작성자 정보도 포함되어 있습니다.
이 코드 패치에 대한 개선 제안은 다음과 같습니다:
- 주석 추가: 각 항목의 역할, 의도 및 사용 예 등을 설명하는 주석을 추가하는 것이 좋습니다.
- 유효성 검사: 입력된 이미지 파일 이름, idiomatic 값 및 scale 값이 유효한지 확인하는 추가적인 유효성 검사를 수행하는 것이 바람직합니다.
- 확장성 고려: 현재 코드는 세 가지 이미지만 포함하고 있으며, 더 많은 이미지를 지원해야 할 경우 유연성이 부족할 수 있습니다. 이에 대비하여 배열을 사용하여 다양한 이미지를 나타낼 수 있는 구조로 변경하는 것이 좋습니다.
위의 제안사항을 고려하여 코드를 개선할 수 있습니다.
|
||
struct UserInfo { | ||
let userNickname: String | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 패치에서는 UserInfo라는 구조체가 정의되어 있습니다. 이 구조체는 userNickname이라는 속성을 가지고 있습니다.
버그 위험은 보이지 않으며, 코드 리뷰나 개선 제안은 다음과 같습니다:
- 파일 상단에 작성자와 작성일을 주석으로 추가하여 코드의 출처와 업데이트된 날짜를 명시적으로 표기할 수 있습니다.
- 주석 아래의 빈 줄들은 삭제하면 코드의 가독성이 향상됩니다.
개선된 코드:
//
// UserInfo.swift
// DontBe-iOS
//
// Created by 변희주 on 2024/01/11.
//
import Foundation
struct UserInfo {
let userNickname: String
}
※ 제 답변은 일반적인 개발 지침에 기반하여 작성된 것입니다. 특정 프로젝트의 요구사항과 코딩 스타일 가이드라인에 따라 조금 다를 수도 있습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
최고 👻👻 고생했습니다 ~!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3
약관동의 + 닉네임 설정까지 모두 구현 고생하셨습니다!!
guard let scalar = UnicodeScalar(self)?.value else { | ||
return false | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3
공백 처리 해주시면 좋을거같아여!
닉네임 특수문자 가능한지, 숫자 1개 당 1글자로 처리할 것인지 요런 거 한번 기획 측에 확인해보면 좋을 것 같아요! |
let consonantScalarRange: ClosedRange<UInt32> = 12593...12622 | ||
return consonantScalarRange ~= scalar | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 패치에는 크게 문제가 없어 보입니다. 다만 개선을 위해 몇 가지 제안 사항이 있습니다:
-
주석: 코드 파일의 상단에 있는 주석은 좋은 접근 방식입니다. 그러나 주석에 한글로 작성하는 대신 영문으로 작성할 것을 권장합니다. 이렇게 하면 코드를 이해하거나 협업하는 데 도움이 될 수 있습니다.
-
변수/상수명: 변수 및 상수명은 식별적이고 의미 있는 이름으로 사용하는 것이 좋습니다. 예를 들어,
isConsonant
대신hasConsonant
나containsConsonant
와 같은 이름을 사용하여 함수의 목적을 더 잘 설명할 수 있습니다. -
예외 처리:
guard
문에서 optional 값을 unwrapping 할 때, 문자열이 비어있을 경우에 대한 처리가 필요합니다. 현재 코드에서는 빈 문자열인 경우UnicodeScalar(self)
호출 시 에러가 발생합니다. 이에 대한 처리를 추가하면 더욱 안전한 코드가 될 수 있습니다. -
범위 연산자: 비교 연산자(
~=
)를 사용하여 문자 값이 자음 범위에 속하는지 확인하는 것은 좋은 접근 방식입니다. 다만, 코드에서 사용된 범위(12593...12622
)에 대해서 주석으로 설명을 추가하는 것이 좋습니다. 자음의 범위가 무엇을 의미하는지 알려주는 것이 기능 확장성과 코드 유지 보수를 위해 도움이 될 수 있습니다. -
단위 테스트: 코드의 품질을 높이기 위해 해당 함수에 대한 단위 테스트를 작성하는 것이 좋습니다. 유효한 문자열과 유효하지 않은 문자열에 대해 테스트를 수행하여 코드가 예상대로 작동하는지 확인할 수 있습니다.
이런 면들을 고려하여 코드를 개선하고 안전성을 확보할 수 있을 것입니다.
@@ -148,7 +148,7 @@ extension OnboardingEndingViewController { | |||
output.voidPublisher | |||
.sink { value in | |||
if value == "start" { | |||
let viewController = OnboardingViewController() | |||
let viewController = DontBeTabBarController() | |||
print(self.introductionView.introduction.text ?? "") // 텍스트 필드 텍스트 잘 넘어오는지 확인 | |||
self.navigationController?.pushViewController(viewController, animated: true) | |||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아래는 코드 패치입니다. 버그 위험과/또는 개선 제안 사항에 대해 간단한 코드 리뷰를 도와드리겠습니다.
-
라인 36:
profileImage
의 속성 중image
가 변경되었습니다. 이전에는ImageLiterals.Onboarding.imgProfile
을 사용하고 있었는데, 이제는ImageLiterals.Common.imgProfile
로 변경되었습니다. 변경된 이미지의 출처를 확인하고, 해당 이미지가 제대로 불러와지는지 테스트하는 것이 좋습니다. -
라인 148:
OnboardingEndingViewController
의 확장(extension)에서output.voidPublisher
를 구독하고 있습니다.value
가 "start"인 경우OnboardingViewController
로 전환되도록 되어 있습니다. 그러나DontBeTabBarController()
로 뷰 컨트롤러를 생성하도록 변경되었습니다. 이 변경사항의 의도와 적절한지 확인하는 것이 좋습니다. 추가로,self.introductionView.introduction.text
의 값이 예상대로 전달되는지 확인하는 부분도 있는데, 텍스트 필드에서 올바른 값을 받았는지 테스트하는 것이 좋습니다.
👻 PULL REQUEST
💻 작업한 내용
💡 참고사항
Don-tBe-iOS/DontBe-iOS/DontBe-iOS/Presentation/Join/ViewControllers/JoinProfileViewController.swift
Lines 130 to 170 in 05dfd22
Don-tBe-iOS/DontBe-iOS/DontBe-iOS/Presentation/Join/ViewControllers/JoinProfileViewController.swift
Lines 93 to 126 in 05dfd22
📸 스크린샷
📟 관련 이슈