Edit Calculator && Use NaverOpenAPI #11

kuk6933 · 2022-09-02T11:50:07Z

Calculator

Model logic에서 가능한 중복을 많이 줄였으나 equal과 setOperator 중복을 처리하는 과정에서 약간 부자연스러움이 있어 이렇게 중복을 줄이는게 있는것 보다는 더 나은 코드인지 여쭤보고싶습니다
HStack과 VStack을 사용하는 대신 Grid를 이용하려 했지만 0의 크기가 다른 점 때문에 아직 해결하지 못했습니다

Network

한가지 API를 사용하는 방법은 실습해봤으나 세개를 한번에 사용하는 네트워크를 구성하는 과정에서 어려움을 겪어 아직 완성하지 못했습니다
세가지 경우일 때 Path설정과 queryItems를 다르게 설정해야 하기에 제너릭스를 이용했지만 이후에 어려움을 겪어 더 세밀한 설계의 필요성을 느꼈습니다
View쪽은 Finder기능 완성과 구조 수정 이후 작성할 예정입니다.
이번주 Network 과제에 시간을 쓰지 못해 부끄러운 PR을 제출하게됐습니다 더 공부해서 계속 수정해보겠습니다
------------>
Network와 viewModel에서 제네릭스를 사용해서 중복되는 코드들을 하나의 함수로 만들어 사용하고 파라미터만 다르게 해서 사용할 수 있도록 하기 위해 매우매우 많은 시간을 고민했지만 해결하지 못했습니다

Network와 Combine은 물론 제네릭스같은 Swift 기능들도 많이 미숙해 공부가 더 많이 필요할 것 같습니다..

- Delete duplicate - Change logic, some naming

swap을 calculatorView에서 동작하도록 해서 ScreenView 권한 최소화

gen-com · 2022-09-03T10:10:16Z

week4/Calculator/Calculator/Style/ButtonStyle.swift

-                    if configuration.isPressed {
-                        Color(white: 1.0, opacity: 0.3)
-                    }
+        configuration.label


상수는 따로 모아서 처리해주면 좋겠네요

gen-com · 2022-09-03T10:14:32Z

week4/Calculator/Calculator/Extensions/String+Extension.swift

-            }
-            return numberFormatter.string(from: NSNumber(value: doubleValue)) ?? self
+        let decimalFractionComponents = components(separatedBy: ".")
+        guard let decimalString = decimalFractionComponents.first,


코드를 그대로 사용한 것이지만, 문법은 통일해 주세요.

guard let safeValue = unsafeValue else { return }

vs

guard let safeValue = unsafeValue else { return }

gen-com · 2022-09-03T10:22:16Z

week4/Calculator/Calculator/Model/Calculator.swift

            }
            return calculationResult / newValue
        case .multiply:
            return calculationResult * newValue
        }
    }
 }
+
+extension Calculator {
+    enum Button: Hashable {


gen-com · 2022-09-03T10:26:14Z

week4/Calculator/Calculator/View/ScreenView.swift

+// MARK: - Constant(s)
+
+extension ScreenView {
+    private enum DrawConstants {


gen-com · 2022-09-03T10:27:30Z

week4/Calculator/Calculator/ViewModel/CalculatorManager.swift

 import SwiftUI

 class CalculatorManager: ObservableObject {

+    // MARK: Alia(es)


gen-com · 2022-09-03T10:29:40Z

week4/Calculator/Calculator/Model/Calculator.swift

-
-    typealias BinaryOperator = CalculatorManager.BinaryOperator
-    typealias Digit = CalculatorManager.Digit
-
    // MARK: Propery(ies)

    private(set) var displayValue = "0"
    private(set) var isAllClear = true
    private var calculationResult: Decimal? = 0
    private var newValue: Decimal? = nil


네이밍 개선하면 가독성을 더 올릴 수 있을 거 같네요.

gen-com · 2022-09-03T10:30:51Z

week5/NaverOpenAPITest/NaverOpenAPITest/Model/BoxOffice.swift

+struct BoxOffice: Searchable {
+    var searchKeyword = ""
+    var elements: [Any] = []
+    var path = "/v1/search/movie.json"


gen-com · 2022-09-03T10:31:18Z

week5/NaverOpenAPITest/NaverOpenAPITest/Model/BoxOffice.swift

+
+struct BoxOffice: Searchable {
+    var searchKeyword = ""
+    var elements: [Any] = []


뭐하는 변수인가요 ? Any ?

gen-com · 2022-09-03T10:35:30Z

week5/NaverOpenAPITest/NaverOpenAPITest/Searchable.swift

+
+import Foundation
+
+protocol Searchable {


초기화 함수와 아래 세 값들이 있다면 검색할 수 있는 건가요 ?

gen-com · 2022-09-03T10:42:45Z

week5/NaverOpenAPITest/NaverOpenAPITest/ViewModel/Finder.swift

+    static let host = "openapi.naver.com"
+}
+
+class Finder<Article: Searchable>: ObservableObject {


아직 모듈이 미완성인 것 같네요.

네트워크를 요청할 때 필수로 있어야 하는 것들을 모아서 처리할 수 있도록 해봅시다.
선택적으로 구성해야할 것은 구현할 수 있도록 빼보세요.
네임스페이스 안에 적절한 멤버들이 있도록 해보세요.

rose6649 · 2022-09-05T05:05:31Z

week4/Calculator/Calculator/Model/Calculator.swift

+        if isEqual {
+            isPreOperatorEqual = true
+        }
+        calculationResult = calculate(`operator`, newValue)
        guard let calculationResult = calculationResult else {
            displayValue = "오류"


"오류"라는 string 값이 많은데,
이거 상수로 두고 모든 부분에 적용시키면 나중에 하나만 변경시켜도 되니까 좀더 관리하기 쉬울것같아요 ㅎㅎ
let errorMessage = "오류"

displayValue = errorMessage 이렇게?

rose6649 · 2022-09-05T05:06:47Z

week5/NaverOpenAPITest/NaverOpenAPITest/Extension/String+Extension.swift

+    init?(htmlString: String) {
+        let option: [NSAttributedString.DocumentReadingOptionKey: NSAttributedString.DocumentType] = [.documentType: .html]
+        guard let htmlData = htmlString.data(using: .utf16),
+              let nsStr = try? NSAttributedString(data: htmlData, options: option, documentAttributes: nil)


줄임말은 지양합시당

rose6649 · 2022-09-05T05:07:47Z

week5/NaverOpenAPITest/NaverOpenAPITest/Model/BoxOffice.swift

+    var elements: [Any] = []
+    var path = "/v1/search/movie.json"
+
+    init() {}


무엇을 위해 존재하는 init() 일까요

rose6649 · 2022-09-05T05:09:18Z

week5/NaverOpenAPITest/NaverOpenAPITest/Model/BoxOffice.swift

+
+    struct Movie: Codable, Identifiable {
+        let attributedTitle: AttributedString?
+        let image: String


image가 string값이 혹시 맞을까요?
만약 image 의 url을 의미하는 거면 imageURL이 더 좋을 것 같아요~

rose6649 · 2022-09-05T05:11:50Z

week5/NaverOpenAPITest/NaverOpenAPITest/ViewModel/Finder.swift

+}
+
+class Finder<Article: Searchable>: ObservableObject {
+    @Published var model = Article()


model 네이밍이 추상적이에요

rose6649 · 2022-09-05T05:13:38Z

week5/NaverOpenAPITest/NaverOpenAPITest/ViewModel/Finder.swift

+        guard let url = urlComponents.url
+        else { return }
+        var urlRequest = URLRequest(url: url)
+        urlRequest.httpMethod = "GET"


httpmethod도 종류별로 관리할 수 있으면 좋을것같아요
string을 직접 집어넣는것은 조금 지양해보도록 해봐요!

rose6649 · 2022-09-05T05:14:34Z

week5/NaverOpenAPITest/NaverOpenAPITest/ViewModel/Finder.swift

+            else { return }
+            DispatchQueue.main.async { [weak self] in
+                self?.model.elements = parsedData.items.indices.map {
+                    BoxOffice.Movie(parsedData.items[$0], id: parsedData.start + $0)


좀더 좋은 방법은 없을까요??

rose6649 · 2022-09-05T05:15:36Z

week5/NaverOpenAPITest/NaverOpenAPITest/ViewModel/Finder.swift

+            }
+        }
+        task.resume()
+        fetchingStatus = .idle


resume()하고 .idle 상태로 보내는 것이 확실히 맞을까요?

rose6649 · 2022-09-05T05:16:13Z

week5/NaverOpenAPITest/NaverOpenAPITest/ViewModel/Finder.swift

+
+    struct News: Codable {
+        let title: String
+        let originallink: String


카멜케이스

rose6649 · 2022-09-05T05:16:41Z

week5/NaverOpenAPITest/NaverOpenAPITest/ViewModel/Finder.swift

+    }
+}
+
+struct Response: Codable {


네이밍이 모호해용

rose6649

알고있을지 모르겠지만 codable하는 모델들의 row한 네이밍들을 codingkey을 이용해 자신이 원하는 네이밍으로 변경할 수 있어요
모른다면 꽤 유용하니 한번 찾아봐요 :D

Finder의 fetch 메서드를 모든 타입(news, movie, document)에서 사용 할 수 있도록 만들고 싶지만 아직 구현하지 못함.

…하지 못함

// matchedGeometryEffect error Refactor

kuk6933 added 3 commits August 29, 2022 17:45

Reflect review

9e04d9d

- Delete duplicate - Change logic, some naming

0.0000~ 계속 입력되는 bug block

1b8972c

swap을 calculatorView에서 동작하도록 해서 ScreenView 권한 최소화

NaverOpenAPI를 이용한 Network학습

6114815

gen-com assigned kuk6933 Sep 2, 2022

gen-com requested review from gen-com, leejun6694 and rose6649 September 2, 2022 15:32

kuk6933 added 3 commits September 3, 2022 10:10

Keep coding convention && Edit naming

1b03827

Change Naming

b272cba

Keep coding convention

7ec500f

gen-com reviewed Sep 3, 2022

View reviewed changes

rose6649 reviewed Sep 5, 2022

View reviewed changes

week5/NaverOpenAPITest/NaverOpenAPITest/ViewModel/Finder.swift Outdated

}

}

struct Response: Codable {

Copy link

rose6649 Sep 5, 2022

Choose a reason for hiding this comment

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

네이밍이 모호해용

rose6649 reviewed Sep 5, 2022

View reviewed changes

kuk6933 added 4 commits September 5, 2022 17:53

Make NetworkManager

8ed199b

Finder의 fetch 메서드를 모든 타입(news, movie, document)에서 사용 할 수 있도록 만들고 싶지만 아직 구현하지 못함.

Search News/ Document // 제네릭스 이용해서 함수 하나로 모든 타입 검색 가능하도록 만들려했지만 아직 구현…

22ecceb

…하지 못함

Change naming, Keep coding convention

09d412b

Apply animation

78f819b

// matchedGeometryEffect error Refactor

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Edit Calculator && Use NaverOpenAPI #11

Edit Calculator && Use NaverOpenAPI #11

kuk6933 commented Sep 2, 2022 •

edited

Loading

gen-com Sep 3, 2022

gen-com Sep 3, 2022

gen-com Sep 3, 2022

gen-com Sep 3, 2022

gen-com Sep 3, 2022

gen-com Sep 3, 2022

gen-com Sep 3, 2022

gen-com Sep 3, 2022

gen-com Sep 3, 2022

gen-com Sep 3, 2022

rose6649 Sep 5, 2022

rose6649 Sep 5, 2022

rose6649 Sep 5, 2022

rose6649 Sep 5, 2022

rose6649 Sep 5, 2022

rose6649 Sep 5, 2022

rose6649 Sep 5, 2022

rose6649 Sep 5, 2022

rose6649 Sep 5, 2022

rose6649 Sep 5, 2022

rose6649 left a comment

Edit Calculator && Use NaverOpenAPI #11

Are you sure you want to change the base?

Edit Calculator && Use NaverOpenAPI #11

Conversation

kuk6933 commented Sep 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rose6649 left a comment

Choose a reason for hiding this comment

kuk6933 commented Sep 2, 2022 •

edited

Loading