-
Notifications
You must be signed in to change notification settings - Fork 178
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
계산기 [STEP 3] Ari #112
계산기 [STEP 3] Ari #112
Conversation
- 숫자입력이 있을 때 숫자를 "0"으로 변경하도록 기능 추가
- 연산자 레이블의 변화가 존재할 때 문자열을 삭제하도록 구현
- 점이 이미 Label에 포함된 경우 더이상 추가하지 않도록 예외사항 구현
- 숫자가 0인 경우 버튼이 작동하지 않도록 구현
- AC Button 클릭 시 Formula Stack View가 비워지도록 기능 구현
- Label의 속성을 설정해주는 setUpFormulaLabel 메소드 추가
- y값 계산을 위해 stackView 추가 후 scrollView의 layout을 즉시 업데이트 하도록 layoutIfNeeded() 호출 - y좌표 계산식을 수정함
- scrollView 안에 추가되어있던 stackView 내부에 Label을 String으로 반환하는 메소드 구현 - 연산 결과를 NumberFormatter로 설정하는 메소드 구현
…로 수정 #5 - UIStackView의 extension으로 확장하고 toString 프로퍼티로 구현
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.
아리! 또 반가워요~ 여기까지 달려오느라 고생이 많았어요.
짧은 시간이었지만 많은 발전들을 보여주었네요!
이번 스텝도 고생 많았답니다~
코멘트 보면서 조금 더 다듬어나가 볼까요~?
layoutIfNeeded()
많이 쓰여요. 정말 많이 쓰입니다, 생각보다.
그렇다고 뭐 guard처럼 많이 쓰인다는건 아니지만... 그래도 생각보다 자주 쓰게 됩니다.
다만, layoutIfNeeded가 어떤것인지 명확하게 이해하고 사용했으면 좋겠어요.
아마 뒤쪽에 수업에서 한번 더 다뤄지긴 할테지만, 지금 궁금해졌다면 한번 찾아보고 테스트 해보는 것을 추천드려요.
메소드 내부가 굉장히 지저분한 것 처럼 보이는데
평소 아리답지 않게... 저도 조금 지저분하다고 느꼈어요.
각각 다른 경우라서 guard를 따로쓴 것 같은데, return 값이 같다면 묶어줘도 상관 없는 것 같아보였어요.
굳이 나눌 정도로 논리적 흐름이 다르다면, 함수로 분리되었어야 하지 않았을까 하는 생각이 드네요..
이 부분 저도 좀 아쉬웠습니다🥲
NumberFormatter
저는 그냥 NumberFormatter 사용합니다. 😉
디버깅 팁
저는 사실 UI 확인할때 하이라키 많이 키고, lldb를 많이 쓰는 편은 아닌거같아요!ㅎㅎ
이 부분에 있어서 저는 팁이 없지만, 아마 제 동기인 그린에게 물어보면 팁을 가득 줄거에요
그는 lldb를 좋아하거든요... 용기내여 그에게 디엠 한 번 해보길...😉
스택뷰 내부에 있는 Label을 꺼내오려고 이중 forEach를 사용하고 있는데, 더 개선해볼 수 있을 지... 조언이 필요합니다.
우리 이러지말고 계산 내역을 스택뷰에 추가할 때, 각각 레이블 두개가 아니라 커스텀 뷰 한 묶음
으로 만들기에 도전해볼까요...!!!!!💪
요번에도 언제든 궁금한게 생기면 DM 쏴주세욥!
@@ -6,13 +6,264 @@ | |||
|
|||
import UIKit | |||
|
|||
// MARK: Property and LifeCycle |
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.
주석의 위치가 다소 애매하다고 생각되어요. 아래 Extension의 Mark들과 같은 위치에 두려고 한 것 같은데, 안쪽에 넣는것이 더 좋지 않을까요?
이 클래스에 대한 설명으로 착각할 수 있겠어요.🥲
currentOperand != "0" | ||
} | ||
|
||
var hasCalculated = 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.
밖에서 접근할 일이 있나요?👀
removeFormulaView() | ||
} | ||
} | ||
// MARK: IBAction method |
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.
보통 MARK 위아래로 줄바꿈을 하는 것 같아요.
아래쪽은 팀, 사람마다 다르지만 위쪽은 다들 하는 것 같은?!
} | ||
|
||
var hasCalculated = false | ||
|
||
override func viewDidLoad() { | ||
super.viewDidLoad() |
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.
왜 super을 호출해야할까요?🤔
없으면 동작하지 않나요?!
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.
viewDidLoad 메소드 내부에 기존 스토리보드에서 추가되어있는 스택뷰와 레이블들을 세팅해주는 메소드를 추가해주었습니다.
즉 viewDidLoad 후에 어떠한 작업을 해주어야하므로 해당 코드를 실행해주려면 super를 호출해야 합니다.
호출하지 않는다면 아직 viewDidLoad의 설정이 끝나지 않은 상태에서 어떤 작업이 이루어지므로 버그 혹은 오동작이 발생할 수 있는 것으로 알고있습니다.
guard hasCalculated == false else { | ||
return | ||
} | ||
guard let newOperand = sender.titleLabel?.text else { | ||
return | ||
} | ||
guard isNotZero || newOperand != "00" else { | ||
return | ||
} | ||
guard isNotZero else { | ||
currentOperand = newOperand | ||
return | ||
} | ||
guard let doubleValue = Double(removeComma(currentOperand) + newOperand) else { | ||
return | ||
} | ||
currentOperand = setUpNumberFormat(for: doubleValue) |
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.
전부 각각 guard를 사용해야하나요?😥
private func removeComma(_ value: String) -> String { | ||
guard value.contains(",") else { | ||
return value | ||
} | ||
return value.replacingOccurrences(of: ",", with: "") | ||
} |
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.
저는 사실 replacingOccurrences만으로 충분히 동작할 것 같은데, 따로 이렇게 만들어준 이유가 있겠죠..?!
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.
따로 쉼표가 문자열에 포함되어있지 않다면 실행하지 않도록 하고 싶었어요!
return value.description | ||
} | ||
guard formatterNumber.count < 26 else { | ||
return formatterNumber.map{ $0.description }[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.
인덱스 0에 접근 하는 것이, 언제나 항상 안전할 수 있을까요?🥲
좀 더 안전하게 배열의 첫번째 값을 꺼내올 수는 없을지요!
guard let input = label?.text else { | ||
return | ||
} | ||
inputValues.append(input.replacingOccurrences(of: ",", with: "")) |
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.
removeComma 웨 않쒀여...? 진짜 궁그매서..
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.
extension UIStackView 부분이라서 바로 removeComma를 가져올 수가 없었어요 😇
그래서 그냥 replacingOccurrences메소드를 사용해주었답니다.
case .wrongOperator: | ||
return "잘못된 연산자입니다." | ||
case .wrongOperand: | ||
return "잘못된 숫자입니다." | ||
case .notNumber: | ||
return "NaN" |
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.
오... 뭔가 description이 느낌이 다르네요🤔
위에서 localizedDescription
을 쓰던데, 이 Error 타입에 적용시켜보는 것은 어떨까요?
@@ -38,6 +38,9 @@ struct Formula { | |||
if operators.isEmpty == false { | |||
operators.clear() | |||
} | |||
guard currentOperand.isNaN == false else { | |||
throw CalculatorError.notNumber |
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.
여기서 봤을땐 흐름상 notNumber보다는 isNaN이 좀 더 명확한 표현이 될수도 있겠다싶네요.
물론 같은 뜻이지만, notNumber이면 int가 아니라 string이야, 혹은 숫자가 아니라 글자가 들어왔어 라는 느낌이 될 것 같아서요..!😀
- UIView extension으로 IBInspectable을 활용하여 cornerRadius 프로퍼티 추가 - 모든 버튼의 cornerRadius 값을 추가
- extension UIView 삭제 - calculatorButtons 배열 추가 - viewWillLayoutSubviews 메소드에 버튼들의 cornerRadius 값을 설정하는 로직 추가
- FormulaStackView 클래스 추가하여 addFormulaStackView 메소드 로직 수정 - extension UIStackView에 내부 로직 수정
- 프로퍼티 isNotCalculated, hasDotNotIncluded, hasMinusNotIncluded 추가 - clearEntryButtonTapped 내부 잘못된 로직 수정
안녕하세요. 엘림! 피드백 보고 개선하면서 궁금했던 것들도 생겨서 추가로 적어보았습니다. 덕분에 코드가 좀더 간결해진 것 같아요. 감사합니다🥰 수정사항
궁금했던 것 / 조언을 얻고 싶은 부분
|
- OSLog를 import 하여 extension 추가 - 에러를 처리하는 부분에 print문이 아니라 os_log문으로 수정
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.
아리~ 역시 제가 생각했던 것보다 잘해주었네요!🙏🙏
좀 더 생각해봤으면 하는 부분만 코멘트로 달았고, 이번 프로젝트는 여기까지 진행하고
뒤 프로젝트에서 다른 캠퍼와 합치면서 여러가지로 다시 고민해보는 것도 좋을 것 같아요.
이번 프로젝트 너무 고생많았고, 계속 발전하는 모습 또 고민하는 모습 보기 좋았습니다👍👍
지금처럼 언제든 편하게 궁금한거 있을때 DM주세요!
(바빠서 빨리 대답 못해줘도 기다려주는 아리 최고...🙏)
다음 프로젝트도 화이팅!
hasCalculated = true | ||
return | ||
case .queueNotFound: | ||
os_log(.error, log: .error, "%@", error.errorDescription ?? error.localizedDescription) |
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.
ㅎㅎ log는 어떤가요? 불편하진 않나요...?🤤
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.
print문 보다 쓰기가 조금 어렵지만 그에 비해 좋은 장점들이 있습니다.
- 위 방식처럼 print문을 상당히 쉽게 대체할 수 있다.
- 콘솔 앱과 로그를 함께 사용하면 보다 효율적인 방식으로 문제를 디버깅하는데 도움이 될 수 있다.
- 성능 오버헤드가 낮고 나중에 검색할 수 있도록 장치에 보관된다.
위와 같은 장점 때문에라도 OSLog를 사용하면 더 좋을 것 같다는 생각이 들었습니다! 😄
import OSLog | ||
|
||
extension OSLog { | ||
private static var subsystem = Bundle.main.bundleIdentifier! |
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.
되도록 강제언래핑은 피해주세요!🙏
default: | ||
return "알 수 없는 에러가 발생했습니다." |
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.
LGTM💚
import UIKit | ||
|
||
class FormulaStackView: UIStackView { | ||
private(set) var element = [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.
LGTM2💚
override init(frame: CGRect) { | ||
super.init(frame: frame) | ||
loadView() | ||
} | ||
|
||
required init(coder: NSCoder) { | ||
super.init(coder: coder) | ||
loadView() | ||
} |
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.
두 init은 어떤 차이가 있을까요?
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.
override 이니셜라이저는 상속한 부모의 이니셜라이저를 재정의한 것이고,
required는 필수로 정의해야하는 이니셜라이저 입니다 😁
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.
coder와 frame의 차이는 이미 알고 있는거라서 말씀 안해주신건가유?😉
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.
헉.. 중요한 부분이였네요. 찾아서 아래와 같이 정리해보았습니다. 😊
init(frame:)
코드로 UIView Class의 View 인스턴스를 만들기 위해 지정된 이니셜라이저.
Frame rectangle을 가지고 구현하고자 하는 뷰의 중심과 경계선을 지정해준다.
스토리보드, xib, nib 같은 인터페이스 빌더를 사용하지 않고 코드로 UIView Calss의 View object를 만들기 위해 지정된 이니셜라이저다.
init(coder:)
기본적으로 storyboard
나 xib
를 활용하면 별도의 코딩 없이 앱의 속성을 수정할 수가 있는데 이것을 가능하게 해주는 과정을 unarchiving
이라고 한다. Interface builder는 코드가 아니기 때문에 앱을 컴파일 하는 시점에서 컴파일러가 인식할 수 없고 이를 코드로 변환해주는 unarchiving 과정이 필요하다는 것이다.
이 과정에서 init?(coder:)
가 사용된다.
파라미터 coder를 통해 NSCoder 타입의 객체가 전달되는 것이고 전달된 NSCoder 타입의 객체가 decoding되어 초기화된 후 컴파일 할 수 있게 decoding된 자기자신(self)을 반환하는 작업이라고 보면 될 것 같다.
내가 구성한 View의 상태를 앱의 disk에 저장하는 과정을 serialize
라고 한다. deserialize는 반대로 disk에 저장된 상태를 불러오는 작업이라고 볼 수 있다.
NSCoding이라는 프로토콜을 통해 이 serialize와 deserialize 작업이 가능해진다.
따라서 init(coder:)의 용도는 아래와 같다.
- Storyboard라는 인터페이스 빌더를 사용하여 뷰의 상태를 수정할 경우 serialization 작업을 Xcode가 init(coder:)를 호출하여 앱 내 뷰 작업을 저장하고 불러오는 작업을 해준다.
UIView 선언부를 보면 NSCoding 프로토콜을 채택하고 있는데 NSCoding 선언부를 살펴보면 실패가능한 이니셜라이저를 작성하도록 되어있다.
프로토콜을 준수하는 클래스에서 프로토콜에서 요구하는 이니셜라이저 요구사항을 구현하려면 required 키워드가 붙어야 한다. 따라서 이를 상속받은 FormulaStackView와 같은 클래스에서는 스토리보드르 사용하고 있지 않지만 init?(coder:)를 구현해줘야 하고 앞에 꼭 required를 붙여주어야 한다.
override func viewWillLayoutSubviews() { | ||
super.viewWillLayoutSubviews() | ||
calculatorButtons.forEach { button in | ||
button.layer.cornerRadius = button.layer.frame.size.width / 2 | ||
} | ||
} |
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.
뷰가 SubView의 레이아웃을 변경하기 직전에 호출되는 메서드입니다!
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.
viewWillLayoutSubviews에 브레이크 포인트 잡아보거나, log 혹은 print 찍어보세요🙏
@lina0322
엘림 안녕하세요!!!
약속했던 기한보다 반나절[?] 일찍 보내드렸...는데 충분히 고민해봤다는 판단에 PR을 보내드립니다. 😆
이 외에 제가 놓친 부분이 있다면 번거롭더라도 한 번만 더 짚어주신다면 감사하겠습니다. 🙏🏻
항상 감사합니다! 엘림 🥰
고민했던 것
자동 스크롤이 제대로 동작하지 않는 경우
![](https://camo.githubusercontent.com/8a0b422e6b28e27dde19bf9da6ca22b085f02e82c088c6147f1eec2cce841b74/68747470733a2f2f692e696d6775722e636f6d2f76344f59464f652e676966)
스크롤 뷰의 콘텐츠 오프셋 설정시 레이아웃을 바로 업데이트 할 수 있도록 하였습니다. 이유는 레이아웃 업데이트가 예약이 되어있는 상태에서 y값 좌표를 계산하여 오프셋설정을 하게된다면 업데이트 이전의 레이아웃으로 y값으로 계산하기 때문에
layoutIfNeeded()
메소드를 호출하여 레이아웃 업데이트가 필요하였습니다.layoutIfNeeded()
메소드를 추가해주고 난 후계산식을 스택뷰에 추가할 때 속성값을 어떻게 설정해줘야하는지 고민해보았습니다. 기존 스토리보드에 설정되어있는 스택뷰의 속성값과 동일하도록 구현해보았습니다.
프로젝트 요구사항 외 추가 설계한 부분
계산이 끝나고나서 추가적으로 발생할 수 있는 예외 핸들링
연산자버튼
혹은AC, CE 버튼
을 제외하고 나머지 버튼들은 동작하지 않도록 구성replacingOccurrences
를 이용하여 제거하는 메소드를 구현hasCalculated
)를 추가하여 다양하게 활용.코드 내부 가독성을 위해 고민했던 것
// MARK:
도 같이 활용해보았습니다.Label.text
는 연산프로퍼티를 활용하여 코드 내부를 개선해보았습니다.+=
연산자 사용을 하지않았습니다. 풀어서 쓰는게 좀더 가독성이 좋을 때가 있다는 생각이 들었습니다.사용하면
stackView.toString
와 같은 결과가 되서 좀더 직관적일 수 있도록 구현해주었습니다.궁금했던 것 / 조언을 얻고 싶은 부분
layoutIfNeeded()
메소드를 즉시 값이 변경되어야하는 애니메이션에서 많이 사용된다고 알고있는데, 그런 경우가 지금 계산기 프로젝트의 자동스크롤 말고 또 어떤 어떤 경우가 있는지 궁금합니다. 또setNeedsLayout()
메소드는 언제 사용되어야 적절한지가 궁금합니다.n
,po
말고 더 상세하게 사용해보지도 못했고, 또 어떻게 연습해봐야할지 감이 잘 안오네요. 현업에 계신 엘림이 자주 사용하는 명령어, 혹은 디버깅을 연습하기 위해서 해보면 좋을만할 것들을 추천받고 싶어요.이 외에 부족한 점이 보이신다면 말씀해주세요! 감사합니다! 🙇🏻♀️