[Downey] SignUpView 구조 재설계 #23

dahun-lee-daji · 2021-04-05T04:27:10Z

쉬는날 PR 요청드려서 죄송합니다 ㅜㅜ
추가 작업 예정입니다. 머지하지 말아주세요!

작업 목록

구조 재설계 : View -> Delegate -> Model -> observer -> View를 구현하려 함.
TextField subClassing
각 textField에 옳은 값이 있을 때만, "다음" button enable
ComplianceChecker 재설계 -> textField의 속성 중 enum textFieldType에 따른 switch-case문은 다형성 구현에 따른 공수에 비한 장점이 적어보여 구현하지 않음.
NetworkManger.postUser(:) method에 Result<>적용.

고민과 해결

기존의 delegate에서 바로 VC로 NotificationCenter.post하여, VC내부에서 판단, UI update 구조 -> delegate에서 판단, 판단내용을 post하여 VC에서 UIUpdate하도록 변경함.
findSameDimensionSignUpTextField(:) 메소드를 generic으로 구현하려 했으나, 실패.
protocol ErrorCheckResult를 준수하는 ErrorResult를 하나의 클래스만 만들었으나, 성공, 실패, none등으로 subclassing하면 더 읽기 쉽지 않을까? 했지만, isError의 값을 고정시키는 것외의 장점이 없다고 생각하여 분리하지 않았음.
NotificationCenter.default.addObservser()의 매개변수 object가 특정 타입을 받도록 구현시도 하였으나, 인스턴스만 받는것으로 확인.

학습키워드

generic, typealias
Result<>

UILabel.changeTextNColor -> .changeStyle configureCategorylabel()의 내용은 storyboard에서 시행 및 코드 삭제

FieldType, TextFormError와 관련된 UILabel을 속성으로 갖도록 정의

기존 설정을 storyboard에서 하도록 수정

textFieldCollection의 SignUpTextField으로 타입 변경에 따른 configureTextField() 내부 속성 설정

TextFormError를 반환하도록 변경. 비밀번호 검사, 이름 isNIl 로직 추가 ComplianceChecker.check의 반환값을 기준으로 UI변경 예정

delegate 내에서 ComplianceChecker를 이용하여 textField 내용 검사 해당 결과를 통해 textField와 참조중인 UILabel의 UI를 변경

정규표현식 비교 결과를 반환하는 method 반환된 결과를 본래 string과 비교하는 method

enum TextFormError으로 관리되던 에러를 protool ErrorCheckResult로 대체하여 다형성 구현

NetworkManager에 싱글톤 패턴 적용. ComplianceChecker에서 NetworkManager 인스턴스 제거 getUserList() 내부의 serialize 기능 분리

observer pattern사용하여 View-> VC로 정보 전달 -> 정보를 바탕으로 UI갱신.

findSameDimensionSignUpTextField 메서드는 generic을 사용하여 downCasting을 시도했으나, 실패함.

godrm

PR을 보냈으니 봐달라는 의미였겠죠? 계속 개선해보세요 ㅎㅎ

godrm · 2021-04-05T06:23:09Z

SignUp/SignUp/Extension/Notification.name+Extension.swift

+extension Notification.Name {
+    static let uiUpdate = Notification.Name.init("uiUpdate")
+    static let nextButtonEnableCheck = Notification.Name.init("nextButtonEnableCheck")
+}


이 노티는 누가 보내는 건가요? 누가 받는건가요?
어느 타입과 관련이 높을까요? 응집도를 높여서 타입에 선언하는 걸 고려해보세요.
extension 자체가 이 노티와 관련된 타입에 있으면 좋습니다.

godrm · 2021-04-05T06:24:09Z

SignUp/SignUp/SignUpView/ComplianceChecker.swift

+        switch textField.textFieldType {
+        case .id:
+            return checkIdTextForm(with: text, closure: closure)
+        case .pw:
+            return checkPwTextForm(with: text)
+        case .pw2:
+            return checkPwDuplicated(with: textField)
+        case .name:
+            return checkNameIsNil(with: text)
+        default:
+            return ErrorResult.init()
+        }


잘 구현하셨습니다만 switch-case구문이 없어도 textField에 따라서 다형성으로 동작하도록 만들 수는 없을까요?

godrm · 2021-04-05T06:25:26Z

SignUp/SignUp/SignUpView/ComplianceChecker.swift

+    private func generateRegexString(target : String, pattern : String) -> String {
+        guard let regex = try? NSRegularExpression(pattern: pattern) else {
+            return ""
+        }
+
+        let regexedTarget = regex.matches(in: target, options: [], range: NSRange(location: 0, length: target.utf16.count))
+        let resultString = regexedTarget.map({
+            String(describing: target[Range($0.range, in: target)!])
+        }).joined()
+
+        return resultString
+    }

    private func regexChekcing(target : String, pattern : String) -> Bool {
-        do {
-            let regex = try NSRegularExpression(pattern: pattern)
-
-            let regexedTarget = regex.matches(in: target, options: [], range: NSRange(location: 0, length: target.utf16.count))
-            let resultString = regexedTarget.map({
-                String(describing: target[Range($0.range, in: target)!])
-            }).joined()
-
-            if resultString == "" {
-                return false
-            }
-
-            if resultString != target {
-                return false
-            }
-        } catch {
-            print(error)
+
+        let resultString = generateRegexString(target: target, pattern: pattern)
+
+        if resultString == "" {
+            return false


HugeLadderGame 예제 기억나나요? 입력에서 값을 검증하고 처리하던 것을 분리했었죠?
여기서 값을 문자열로 바꾸고 regex로 확인하는 과정이 ComplianceChecker 속성과 관련이 없으니 분리해서 별도로 테스트할 수 있는 public 함수가 되어도 되겠죠?

godrm · 2021-04-05T06:26:04Z

SignUp/SignUp/SignUpView/ComplianceChecker.swift

+
+    private func checkNameIsNil(with text: String) -> ErrorCheckResult {
+        return text == "" ? ErrorResult.init(isError: true, message: ErrorMessage.nameIsNil.rawValue) : ErrorResult.init(isError: false, message: "")
    }


check-로 시작하는 함수들 모두 꼭 여기에 구현해야 하는 것은 아닌 것 같습니다.
매개변수나 리턴타입과 관련이 높지 않을까요?

godrm · 2021-04-05T06:27:05Z

SignUp/SignUp/SignUpView/ErrorCheckResult.swift

+    case idIsOk = "사용 가능한 아이디입니다."
+    case idIneligible =  "5~20자의 영문 소문자, 숫자와 특수기호(_,-)만 사용 가능합니다."
+    case idUsed = "이미 사용중인 아이디입니다."
+    case pwIsOk = "안전한 비밀번호입니다."
+    case pwNoNumber = "숫자를 최소 1자 이상 포함해주세요."
+    case pwNoSpecialCharacter = "특수문자를 최소 1자 이상 포함해주세요."
+    case pwNoUpperCase =  "영문 대문자를 최소 1자 이상 포함해주세요."
+    case pwOutOfIndex =  "8자 이상 16자 이하로 입력해주세요."
+    case pw2IsOk = "비밀번호가 일치합니다."
+    case pw2Different = "비밀번호가 일치하지 않습니다."
+    case nameIsNil = "이름은 필수 입력 항목입니다."


모든 에러 메시지를 다 선언하려고 하기 보다는 관련이 있는 에러끼리 묶는 것도 생각해보세요.
혹시 id, pwd 라고 붙어있는게 서로 관련이 있는게 아닐까요?

godrm · 2021-04-05T06:31:08Z

SignUp/SignUp/SignUpView/TextFieldDelegate.swift

-        case "NameTextField":
-            NotificationCenter.default.post(name: .nameTextField, object: textField)
-        default:
+    let complianceChecker = ComplianceChecker()


기존 구조와 다르게 변경했네요. 값을 확인하는 ComplianceChecker 구체 타입에 의존하는 것이 아쉽습니다.
이런 타입이 필요하다면 생성할 때 필요한 인터페이스(프로토콜)를 넘겨주는 게 좋았을 것 같습니다.

godrm · 2021-04-05T06:31:47Z

SignUp/SignUp/SignUpView/TextFieldDelegate.swift

+    let complianceChecker = ComplianceChecker()
+
+    func textFieldDidChangeSelection(_ textField: UITextField) {
+        guard let textField = textField as? SignUpTextField else {


SignUpTextField가 아니어도 여기로 들어올 수 있는건가요?

godrm · 2021-04-05T06:33:56Z

SignUp/SignUp/SignUpView/TextFieldDelegate.swift

+        let checkResult = complianceChecker.check(target: textField, closure: { errorChecker in
+            NotificationCenter.default.post(name: .uiUpdate, object: textField, userInfo: ["result":errorChecker])
+        })
+
+        NotificationCenter.default.post(name: .uiUpdate, object: textField, userInfo: ["result":checkResult])


이런 비교자체를 델리게이트가 판단할 것인지 상위로 전달해서 처리하도록 할 것인지 고민해야 합니다.
델리게이트가 화면 업데이트가 필요하다고 판단하고 notification을 보내는 게 아니라
UITextField 델리게이트는 특정한 TextField가 물어본 textFieldDidChangeSelection()에 대해서만 처리하면 되는거죠.
델리게이트는 자기가 판단할 것만 판단하고, 이게 발생했다는 것을 다른 객체로 이벤트를 전달해서 거기서 화면을 업데이트할 지 말지 판단하는 게 좋습니다.

godrm · 2021-04-05T06:34:44Z

SignUp/SignUp/SignUpView/TextFieldDelegate.swift

-            guard let textField = item else {
+    private func findSameDimensionSignUpTextField(target : UIView) -> [SignUpTextField] {
+
+        let optionalFieldArray = target.superview?.subviews.filter({


채널에 올라왔던 내용을 여기서 사용하고 있네요.
델리게이트가 모든것을 처리하라는 의미가 아닙니다. 특히나 델리게이트가 이런 메소드를 갖고 있는 것 안 좋습니다.

godrm · 2021-04-05T06:36:40Z

SignUp/SignUp/SignUpView/View/SignUpTextField.swift

+enum TextFieldType : Int {
+    case id
+    case pw
+    case pw2
+    case name
+}
+
+class SignUpTextField: UITextField {
+    var relatedLabel : UILabel?
+    var textFieldType : TextFieldType?


세부 타입을 값으로 갖고 있는 것은 전형적인 추상 데이터 타입이네요.
각각 인스턴스가 생기거나 각각 클래스로 만들어서 서로 독립적인 동작을 하도록 만드는 게 좋습니다.
이렇게 만들면 어딘가에서 TextFieldType 값에 따라 switch-case를 비교해서 동작하는 코드가 생기게 됩니다.
그런 방식을 지양하세요. 서브클래스보다 복잡하고 좋지 않은 방식입니다.

dahun-lee-daji added 15 commits April 1, 2021 12:32

불필요 코드 삭제, method명 변경

eb25db1

UILabel.changeTextNColor -> .changeStyle configureCategorylabel()의 내용은 storyboard에서 시행 및 코드 삭제

Custom TextField 정의

bb74bbc

FieldType, TextFormError와 관련된 UILabel을 속성으로 갖도록 정의

InfromationLabel 속성 설정 메소드 제거

fed1c7b

기존 설정을 storyboard에서 하도록 수정

메소드 명 변경, IBOutletCollection 타입 변경

3268fbc

textFieldCollection의 SignUpTextField으로 타입 변경에 따른 configureTextField() 내부 속성 설정

ComplianceChecker로직 변경

0296125

TextFormError를 반환하도록 변경. 비밀번호 검사, 이름 isNIl 로직 추가 ComplianceChecker.check의 반환값을 기준으로 UI변경 예정

observer pattern 삭제

9f47314

delegate 내에서 ComplianceChecker를 이용하여 textField 내용 검사 해당 결과를 통해 textField와 참조중인 UILabel의 UI를 변경

Observer Pattern 잔여 코드 삭제

f63b565

불필요한 코드 제거

90f53e9

정규 표현식 메소드 분리

7cd6c62

정규표현식 비교 결과를 반환하는 method 반환된 결과를 본래 string과 비교하는 method

폴더정리, 에러의 다형성 구현

88f6c3f

enum TextFormError으로 관리되던 에러를 protool ErrorCheckResult로 대체하여 다형성 구현

NetworkManager: Singleton, serialize 기능 분리

1b15ca0

NetworkManager에 싱글톤 패턴 적용. ComplianceChecker에서 NetworkManager 인스턴스 제거 getUserList() 내부의 serialize 기능 분리

네트워크 요청에서 판단 로직 분리

02f9dc6

postUser Result<>적용

b51b618

UI Update 주체 변경

a34f127

observer pattern사용하여 View-> VC로 정보 전달 -> 정보를 바탕으로 UI갱신.

반복되는 내용 메서드화

d0d9a00

findSameDimensionSignUpTextField 메서드는 generic을 사용하여 downCasting을 시도했으나, 실패함.

godrm reviewed Apr 5, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Downey] SignUpView 구조 재설계 #23

[Downey] SignUpView 구조 재설계 #23

Uh oh!

dahun-lee-daji commented Apr 5, 2021 •

edited

Loading

Uh oh!

godrm left a comment

Uh oh!

godrm Apr 5, 2021

Uh oh!

godrm Apr 5, 2021

Uh oh!

godrm Apr 5, 2021

Uh oh!

godrm Apr 5, 2021

Uh oh!

godrm Apr 5, 2021

Uh oh!

godrm Apr 5, 2021

Uh oh!

godrm Apr 5, 2021

Uh oh!

godrm Apr 5, 2021

Uh oh!

godrm Apr 5, 2021

Uh oh!

godrm Apr 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

[Downey] SignUpView 구조 재설계 #23

Are you sure you want to change the base?

[Downey] SignUpView 구조 재설계 #23

Uh oh!

Conversation

dahun-lee-daji commented Apr 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

작업 목록

고민과 해결

학습키워드

Uh oh!

godrm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dahun-lee-daji commented Apr 5, 2021 •

edited

Loading