# PR - 계산기2 MINT & Whales 🧮

###### tags: `커리어` `iOS` `Swift` `계산기2`

## 🏦계산기2 _ 웨일's 민트🧮

# STEP 1
## 🤹🏻 고민했던 점
> ### `Linked List` vs `Double Stack`
스텝 2의 `ExpressionParser` 타입, `Formula` 타입, `String`의 `split(with:)` 메서드의 구현은 둘 다 같은 방법을 사용했어서 비교하여 선택할 필요가 없었습니다. 그러나 스텝 1의 경우 웨일은 `linked list`를, 민트는 `Double stack`을 사용하여 선택에 대한 논의가 있었습니다. 웨일은 `append`와 `removeFirst` 메서드의 시간복잡도가 `O(1)`으로 낮아서 `linked list`를 구현하였고 민트는 `linked list`의 가장 큰 장점인 중간 삽입을 `queue`에서는 사용하지 않고, `linked list` 구현을 위해 `node` 타입을 추가로 구현하는 것이 불필요한 메모리를 사용하는 것 같아서 `Double Stack`을 구현하였습니다. 논의 끝에 `Double Stack`은 `dequeue`를 위해 `reversed()`를 사용할 때 시간 복잡도가 `O(n)`이 되므로 이를 줄이기 위해서, 그리고 `Queable protocol`을 활용해 SOLID를 신경 쓴 부분에서 `linked list`가 더 나은 방법이라고 생각해 이것으로 선택하였습니다.

> ### `PR & merge` 과정
PR로 코드를 합치는 과정에서 약간의 버벅임이 있었습니다. `fork`를 해야하는 건지, 브랜치를 새로 파야하는 건지에 대해 고민하였습니다. `collaborate`로 등록한 `repository`에서 새롭게 합칠 type 단위별로 브랜치를 만들어서 계산기 2의 step1을 위한 브랜치로 PR을 보내 `merge` 하여 프로젝트를 합쳤습니다.

<br>

## 🎙️ 조언을 얻고 싶은 점
연산자 버튼을 터치했을 때 스택뷰가 쌓이도록 코드를 구현하면서 새로운 스택뷰를 반복적으로 만들어주는 것을 보고 `makeCurrentFormulaLabelStackView` 라는 메서드를 따로 만들어 호출하는 형식으로 진행하였습니다. 그랬더니 `CalculateViewController`의 내용이 점점 길어져서 이 부분을 좀 더 보완하고 싶은데 어떤 방법이 있을까요? 조언 주시는 방향 등을 추가 공부하여 `step2 리팩토링` 때 좀 더 좋은 코드를 만들고 싶습니다!!!

# STEP 1 답변
반갑습니다 웨일! 민트! 다시 두분의 코드를 리뷰하게 되어 영광입니다. 짧은시간이지만 화이팅해서 진행해보자구요! 😁

먼저, 두 프로젝트 병합하시느라 고생 많으셨습니다. 생각보다 쉽지 않죠? 😅
병합 경험이 두 분께 새로운 경험이 되셨으리라 생각합니다.

테스트까지 잘 병합해주셨고, 앱도 문제없이 잘 돌아가는거 같군요!
아마 두분의 코드를 합치면서 어디선가 버그가 생길 수도 있습니다. (사실 하나 발견했어요 ㅎㅎ)

버그는 다음 스탭에서 찬찬히 고쳐나가는걸로 하죠! 두분 고생많으셨습니다. 다음 스탭 진행해주세요~

아래는 질문주신 부분에 대한 제 생각입니다! 😁

> 연산자 버튼을 터치했을 때 스택뷰가 쌓이도록 코드를 구현하면서 새로운 스택뷰를 반복적으로 만들어주는 것을 보고 makeCurrentFormulaLabelStackView 라는 메서드를 따로 만들어 호출하는 형식으로 진행하였습니다. 그랬더니 CalculateViewController의 내용이 점점 길어져서 이 부분을 좀 더 보완하고 싶은데 어떤 방법이 있을까요? 조언 주시는 방향 등을 추가 공부하여 step2 리팩토링 때 좀 더 좋은 코드를 만들고 싶습니다!!!

좋은 고민을 하고 계시네요! 👍
사실, 해당 부분은 현재 앱의 구조상 어쩔 수 없는 고질적인 문제입니다.
하지만 그렇다고 어쩔 수 없다고 넘어갈 일은 아니죠? 분명 여러 방법들이 존재합니다.
현재 CalculateViewController가 하고 있는일을 잘 생각해보세요!
- 사용자의 입력 받기
- 각각 받아온 입력을 상황에 맞게 가공하기
- 가공된 입력을 토대로 계산을 요청하거나, 새로운 뷰를 만들어 넣기

다 적지는 못하겠지만 많은 일을 하는건 확실하군요..! 🤔
받아온 입력을 가공하는 로직이나 계산하는 로직을 다른 객체를 만들어 그 일을 대신하게 한다면 어떨까요? 그러면 해당 로직들이 넘어가면서 조금은 가벼워지지 않을까요? 한번 고민해보시기 바랍니다..!(이 부분은 이번 프로젝트에서 적용하실 필요는 없습니다. 공부만 해보세요!)

> 추가적으로 다음 스탭에서 리팩토링 할 때 다음 부분은 꼭 참고해보세요!

- 소수점을 입력 후 00 버튼을 누르면 소수점이 사라지는 문제
- 4자리 이상 숫자를 입력했을 때 스택뷰에 단위를 나눠주는 콤마가 사라져서 올라가는 문제
- checkOperandForm(_:) 메서드의 반환값으로 "error"스트링을 반환하는건 조금 문제가 있어보이네요..! 좀 더 좋은 방법으로 고민해봅시다.

---

# STEP 2
## 🤹🏻 고민했던 점
### 🔹`UILabel`과 `UIStackView`의 `ViewController`로부터의 분리
`ViewController`에 너무 많은 내용이 기재되어 있어 어떠한 로직들을 분리할 수 있는지에 대한 고민이 있었습니다. 그 중에서 `subStackView`를 밖으로 뺄 수 있지 않을까? 싶어서 찾아보았습니다. 두가지 방법을 찾을 수 있었습니다.
1. `extension`으로 `convenience init`을 사용하여 조건들을 설정해주는 방법
2. `Custom Stack View`를 구현하는 방법
그 중에서 `extension`을 사용하는 방법이 더 저희의 이해도에 맞는 것 같아 이것으로 구현하였습니다.

extension UILabel {
    convenience init(text: String, font: UIFont = .preferredFont(forTextStyle: .title2), textColor: UIColor = .white) {
        self.init(frame: .zero)
        self.text = text
        self.font = font
        self.textColor = textColor
    }
}

extension UIStackView {
    convenience init(firstLabel: UILabel, secondLabel: UILabel, spacing: CGFloat = 8, alignment: Alignment = .bottom) {
        self.init(frame: .zero)
        self.spacing = spacing
        self.alignment = alignment
        self.addArrangedSubview(firstLabel)
        self.addArrangedSubview(secondLabel)
    }
}

`convenience init`은 보조적인 것으로 필요한 경우 생성하여 사용할 수 있습니다. 이때 `self` 키워드를 붙여주기 위해 `self.init`이 필요합니다. `(frame: .zero)`는 기본값으로 빈 것을 나타냅니다.

이를 사용하여 `ViewController`에서 `stackView`와 `Label`을 구현하는 과정을 분리할 수 있었습니다.

<br>

### 🔹`Button method`의 `ViewController`로부터 분리
위의 이유와 똑같이 `ViewController`에서 버튼이 눌려질 때 해야하는 일들과 검증해야하는 조건들을 다른 `Manager` 객체로 분리하였습니다. 옵셔널을 반환값으로 주어 `nil`인 경우는 `ViewController`에서 그 버튼을 누른 경우 `return`이 되게 하였고, 값이 있는 경우는 그 값을 `label`의 `text`로 입력해 주었습니다.

이렇게 변경하였더니 수식을 추가하는 `addCurrentFormula` 메서드에 대해서도
- `addCurrentFormula` : `ViewController`에서는 `StackView`에 수식이 추가되는 것이고

private mutating func addFormula(_ currentLabelText: String, _ buttonText: String = "") {
    let operandText = FormManager.transformResult(from: (currentLabelText)).replacingOccurrences(of: ",", with: "")

    formulasUntilNow += " \(buttonText) \(operandText) "
}

- `addCurrentFormulaOnView` : `CalculatorManager`에서는 `parse`에 넣을 문자열에 수식이 추가되는 것으로 분리하여 바꿀 수 있었습니다. 전체적으로 조금 더 `Controller`와 `Model`의 역할에 맞게 분리할 수 있었습니다.

private func addCurrentFormulaOnView() {
        guard let operatorLabelText = currentOperatorLabel.text,
              let operandLabelText = currentOperandLabel.text else {
            return
        }
        
        setCurrentFormulaViewOnScroll(operatorLabelText, FormManager.transformResult(from: operandLabelText))
    }

- `ViewController`의 `OperandsButton`에서는 `CalculatorManager`에 `View`로부터 입력된 값을 넘기기만 한 후 그 결과를 옵셔널 바인딩만 진행하여 버튼 `action`을 성공하게 할지 안할지 결정합니다.

@IBAction func tappedOperandsButton(_ sender: UIButton) {
        guard let number = sender.currentTitle,
              let operandLabelText = currentOperandLabel.text,
              let labelText = calculatorManager.verifyButton(for: number, currentLabel: operandLabelText) else {
            return
        }
        
        currentOperandLabel.text = labelText
    }

- `Calculator Manager` 에서는 `ViewController`로부터 값을 전달받아 조건에 따라 `nil`이나 양식에 맞게 변환한 값을 반환합니다.

private func verifyOperandLabel(_ currentLabelText: String, _ buttonText: String) -> String? {
        guard isCalculated == false,
              (currentLabelText + buttonText).count <= 20 else {
            return nil
        }
        
        guard currentLabelText != "0" else {
            return buttonText
        }
        
        return FormManager.transformResult(from: currentLabelText + buttonText)
    }

<br>

### 🔹`NumberFormatter` -> 네임스페이스의 프로퍼티로 수정
계산기 요구사항을 맞추기위해 `NumberFormatter`를 사용하는 데에 있어서 민트와 웨일의 방법이 달랐습니다.
- 민트는 직접 넣을 수 있도록 `String`을 매개변수로 받아서 `String` 타입을 반환하는 메서드로 구현하였고

private func formattingNumber(_ input: String) -> String {
    let formatter = NumberFormatter()
    let number = NSDecimalNumber.init(string: input)

    formatter.maximumSignificantDigits = 15
    formatter.numberStyle = .decimal
    formatter.roundingMode = .halfUp
    formatter.usesSignificantDigits = true
        
    return formatter.string(from: number) ?? "NaN"
}

- 웨일은 `NumberFormatter` 자체의 메서드를 불러오도록 `NumberFormatter` 타입을 반환하는 메서드로 구현하였습니다.

func formatter() -> NumberFormatter {
    let numberFormatter = NumberFormatter()

    numberFormatter.numberStyle = .decimal
    numberFormatter.roundingMode = .halfUp
    numberFormatter.maximumFractionDigits = 20

    return numberFormatter
}

- 최종적으로 네임스페이스를 이용해 `NumberFormatter`를 만들어주면 인스턴스 생성도 필요하지 않고 연산 프로퍼티를 이용해 구현하여 로직을 `ViewController`에서 분리할 수 있어서 효율성 측면에서도 가독성 측면에서도 좋다고 생각이 되어 아래와 같이 코드를 구현하였습니다.

enum FormManager {
    static let numberFormatter = NumberFormatter()
    
    static var configuredNumberFormatter : NumberFormatter {
        self.numberFormatter.numberStyle = .decimal
        self.numberFormatter.maximumFractionDigits = 19
        self.numberFormatter.maximumIntegerDigits = 20
        
        return self.numberFormatter
    }
}

<br>

### 🔹`weak` 키워드를 이용한 `removeFirst` 메서드 수정
- `removeFirst` 메서드를 구현할 때 고려했던 부분이
1. 비어있다면 `nil` 반환
2. 노드가 하나라면 `head`와 `tail` 모두 `nil`을 주고 원래 `head`의 `data` 반환
3. 그 외의 경우는 `head`를 두 번째로 넘기고 `count` 하나 줄이며 원래 `head`의 `data` 반환

이렇게 세 가지였습니다.

    private var tail: Node<T>?
    private(set) var count: Int = 0

    mutating func removeFirst() -> T? {
        guard !isEmpty else {
            return nil
        }

        let data = head?.data

        if count == 1 {
            head = nil
            tail = nil
            count = 0
        } else {
            head = head?.next
            count -= 1
        }

        return data
    }

- 리팩토링을 진행하면서 보니 `count`라는 프로퍼티가 뚜렷하게 필요한 부분이 보이지 않아 삭제하고 싶었고 이에 따라 문제가 되는 부분이 `removeFirst`메서드 뿐이었습니다.
노드가 하나일 때 실행되는 로직을 보면 `head`를 `nil`로 바꿔주는 부분은 `head?.next`와 일맥상통하고, `count`는 지워줄거라 문제가 되지 않는데 `tail`에 대한 고민이 많았습니다. `tail`을 직접 `nil`로 바꿔주지 않는다면 계속 메모리 상에 살아있다가 나중에 새로 노드를 만들어줄 때 해제되기 때문에 `메모리적 낭비`가 생긴다고 생각하였습니다. 그러다 선택한 방법이 `weak` 키워드였습니다. `weak` 키워드를 이용해 약한 참조를 하면 `ARC`로 인해 자동으로 `tail`의 노드가 해제되는 방법으로 해결할 수 있었습니다.

    private weak var tail: Node<T>?

    mutating func removeFirst() -> T? {
        let data = head?.data

        head = head?.next

        return data
    }

<br>

## 🎙️ 조언을 얻고 싶은 점
### 🔹linked list와 queue의 역할 구분
현재 저희 코드에서는 `linked list`와 `queue`가 따로 구현되어 있습니다. 그런데 계산기의 `queue`에서는 사용하지 않는 `linked list`만의 함수들이 있길래 이 프로젝트에서는 불필요한 느낌이라 삭제하였습니다. 그런데 이 경우 `queue`와 `linked list`에 구현되어 있는 역할이 거의 비슷하기에 두 객체의 역할이 구분되지 않는 느낌입니다. 이러한 경우 그냥 `queue`와 `linked list`를 하나로 구현하는 것이 좋을까요? 아니면 `linked list`에 기존 `linked list`만의 함수, 예를 들어 `insert` 와 같은 것들을 구현해 놓는 것이 더 좋을까요?

<br>