Skip to content
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

계산기II [STEP1] Diana, Gray #581

Open
wants to merge 17 commits into
base: ic_11_diana
Choose a base branch
from

Conversation

yawoong2
Copy link

안녕하세요! @uuu1101 🤗
다름이 아니라, 계산기II Step1이 완료되어 PR 드립니다!

이번 프로젝트는 저희 둘이 디스코드로 의사소통하며 코드 병합을 진행해 보았습니다.
그 와중에 한 쪽에서 commit을 도맡아 작성하게 되어서 모두 Gray의 commit으로 기록되어 있습니다. 이 점 참고 부탁드립니다!

1. 고민이 되었던 부분 🤔

  • 시간복잡도를 고려한 메서드 선택
// String.swift
// 1안
extension String {
    func split(with target: Character) -> [String] {
        return self.components(separatedBy: String(target))
    }
}

// 2안
extension String {
    func split(with target: Character) -> [String] {
        return self.split(separator: target).map { String($0) }
    }
}

저희가 내린 결론은 components 메서드를 활용한 코드는 target을 미리 String으로 형변환한 후, components seperatedBy를 수행하여 O(n) 입니다.
그러나 split 메서드를 활용한 코드는 split를 수행하여 O(n)만큼 시간복잡도가 발생하고, 그 결과값으로 map 메서드를 수행하여 O(n)만큼의 시간복잡도가 더해집니다.
따라서, components 메서드를 활용해서 구현한 코드로 병합했습니다.

  • 실패가능한 초기화
// ExpressionParser.swift
enum ExpressionParser {
    static func parse(from input: String) -> Formula {
.
.
        input
            .compactMap { Operator(rawValue: $0) }
            .forEach { operators.enqueue($0) }
.
.
    }
}

위 코드에서 .compactMap { Operator(rawValue: $0) } 는 Operator의 rawValue를 통한 Initializer가 실패가능하다는 것을 알게되어 위와 같이 작성했습니다.
Operator의 케이스에 해당하지 않는 문자열은 실패가능한 Initializer로 nil을 반환할 수 있기 때문에, compactMap 메서드를 통해서 nil은 제외하고 변환이 가능한 값으로 배열을 만들 수 있었습니다.

2. 조언을 얻고싶은 부분 ❓

프로젝트의 요구사항에 따라 PR을 통해 코드를 병합해 보았는데, 하나의 repository 내에서 step3 -> main으로 PR을 통해 병합하는 것이 의도에 맞게 진행한 것인지 궁금합니다!

@uuu1101 uuu1101 self-requested a review February 24, 2024 04:31
Copy link
Member

@uuu1101 uuu1101 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yawoong2 @Diana-yjh

계산기 프로젝트 하시느라 고생하셨어요 !
전체적으로 코드 잘 작성해주신것 같습니다.
학습해시면 좋을것 같은게 있어서 코멘트 남겨두었어요.

  • 커밋 같은 경우에는 다음 프로젝트부터는 한분이 맡아서 하시는거보다 git을 익힌다는 마음으로
    같이 번갈아가면서 해보시면 좋을 것 같습니다 ㅎㅎ

  • 파일 분리도 잘해주셨네요 ! 👍🏻


var isEmpty: Bool {
head == nil
}

private var count: Int {
var result = 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 부분 코드는 Property Observer를 사용할 수 있을 것 같아요!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants