Refactoring #1
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,16 @@ | ||
| // | ||
| // Command.swift | ||
| // MVVM_Calculator | ||
| // | ||
| // Created by IJ . on 2020/09/12. | ||
| // Copyright © 2020 jun. All rights reserved. | ||
| // | ||
|
|
||
| enum Command: String { | ||
|
Collaborator
Author
There was a problem hiding this comment. result, initialize가 연산자(OperationType)인지 생각해볼 필요가 있습니다. 이들은 연산자가 아니기 때문에 좀 더 넓은 의미의 Command라는 이름으로 바꿨습니다. |
||
| case plus = "+" | ||
| case minus = "-" | ||
| case multiply = "×" | ||
| case divide = "÷" | ||
| case result = "=" | ||
| case initialize = "AC" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| // | ||
| // CalculatorModel.swift | ||
| // MVVM_Calculator | ||
| // | ||
| // Created by IJ . on 2020/09/11. | ||
| // Copyright © 2020 jun. All rights reserved. | ||
| // | ||
|
|
||
| protocol CalculatorModelDelegate: class { | ||
|
Collaborator
Author
There was a problem hiding this comment. 모델에 대한 델리게이트 프로토콜을 선언했습니다. 이 프로토콜을 구현하는 객체는 모델의 이벤트를 받아 적절하게 처리하게 될 것입니다. |
||
| func calculatorModelDidChangeNowValue(with value: Int) | ||
| } | ||
|
|
||
| struct CalculatorModel { | ||
| private(set) var beforeValue: Int = 0 | ||
| private(set) var nowValue: Int = 0 { | ||
|
Collaborator
Author
There was a problem hiding this comment. 기존에 getNowValue()나 getBeforeValue() 같은 메소드가 있었습니다. 이렇게 해줄 필요 없이 private(set) 접근 지정자를 붙여서 밖에서는 접근만 가능하도록 할 수 있습니다. |
||
| didSet { | ||
| delegate?.calculatorModelDidChangeNowValue(with: nowValue) | ||
| } | ||
| } | ||
|
|
||
| weak var delegate: CalculatorModelDelegate? | ||
|
|
||
| private var beforeCommand: Command? | ||
| private var hasCommandUpdated: Bool = false | ||
|
Collaborator
Author
There was a problem hiding this comment. 기존에는
|
||
|
|
||
| mutating func setBeforeValue(_ beforeValue: Int) { | ||
|
Collaborator
Author
There was a problem hiding this comment. 이것도 그렇고 다른 set 메소드들에 대해서, 메소드 이름에서 '어떤 것'을 설정하는지 나타내고 있었으나 인자 레이블에서 이를 한번 더 언급하고 있었습니다. ( 이런 경우 인자 레이블을 생략해도 문제 없을 것입니다. |
||
| self.beforeValue = beforeValue | ||
| } | ||
|
|
||
| mutating func setNowValue(_ nowValue: Int){ | ||
| if hasCommandUpdated { | ||
| self.nowValue = nowValue | ||
| } else { | ||
| self.nowValue = self.nowValue * 10 + nowValue | ||
| } | ||
|
|
||
| if hasCommandUpdated { | ||
| hasCommandUpdated = false | ||
| } | ||
| } | ||
|
|
||
| mutating func setCommand(_ command: String) { | ||
| guard let command = Command(rawValue: command) else { return } | ||
|
Collaborator
Author
There was a problem hiding this comment. 기존에 if문을 사용했고 else절은 없었습니다. if문을 사용하면 들여쓰기 수준이 하나 높아진 상태로 코드를 작성하게 됩니다. 이런 경우 guard문을 사용하면 코드를 좀 더 읽기 쉽게 할 수 있습니다. |
||
|
|
||
| switch command { | ||
| case .plus: | ||
| performOperatorCommand(.plus) | ||
| case .divide: | ||
| performOperatorCommand(.divide) | ||
| case .minus: | ||
| performOperatorCommand(.minus) | ||
| case .multiply: | ||
| performOperatorCommand(.multiply) | ||
| case .result: | ||
| guard let beforeCommand = beforeCommand else { return } | ||
|
|
||
| switch beforeCommand { | ||
| case .plus: | ||
| nowValue = beforeValue + nowValue | ||
|
Collaborator
Author
There was a problem hiding this comment. 기존에 self가 다 붙어있었습니다. self를 사용하는 것에 대해 일관된 컨벤션을 갖는 것을 추천합니다. 사용하는 것을 컨벤션으로 해도 문제 없습니다. 일관된 컨벤션을 가지기만 하면 됩니다. |
||
| case .divide: | ||
| nowValue = beforeValue / nowValue | ||
| case .minus: | ||
| nowValue = beforeValue - nowValue | ||
| case .multiply: | ||
| nowValue = beforeValue * nowValue | ||
| default: | ||
| print("4칙연산 이외의 케이스 감지 ERROR") | ||
|
Collaborator
Author
There was a problem hiding this comment. Command에 연산자도 포함되어 있어서 발생하는 문제라고 생각합니다. 연산자와 기타 커맨드(AC, =)를 구분해서 코드를 작성하면 이런 코드는 나오지 않을 거라 생각합니다. |
||
| } | ||
| case .initialize: | ||
| initialize() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Private Method | ||
|
|
||
| private extension CalculatorModel { | ||
| mutating func performOperatorCommand(_ command: Command) { | ||
|
Collaborator
Author
There was a problem hiding this comment. 기존 네이밍은 연산자 명령어가 들어왔을 때 어떤 작업을 수행하고 있었으므로 |
||
| setBeforeValue(nowValue) | ||
| beforeCommand = command | ||
| hasCommandUpdated = true | ||
| } | ||
|
|
||
| mutating func initialize() { | ||
| nowValue = 0 | ||
| beforeValue = 0 | ||
| } | ||
| } | ||
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.
상속되지 않는 클래스에 대해 final class로 선언하는 것은 성능상 이점이 있습니다.
관련 내용은 검색해보시고, 접근 수준에 대해서 최대한 낮은 수준(private)부터 부여한 후 높여가는 것처럼, 클래스 선언시에도 final class로 선언하여 상속 불가능한 클래스로 만들고, 나중에 해당 클래스를 상속받을 필요가 있을 때 final을 풀어주는 식의 습관을 들이는 것을 추천드립니다.