AI 챗봇 앱 [STEP 2] 토미, 이지 #42

angryeon7 · 2024-04-04T02:31:37Z

안녕하세요 아샌! 요즘 날씨가 너무 좋네요..! 봄이에요☀️
Alamofire를 사용하기 위해서 URLSession부터 차근차근 공부한 후에 구현하게되면서 PR를 늦게 보내게되었네요...이점 정말 죄송합니다😢
편하게 피드백 주세요!

🔍 What is this PR?

네트워크 통신 개선

네트워크 통신을 개선하고 Alamofire를 도입하여 코드를 간소화하고, 네트워크 요청과 관련된 로직과 채팅 서비스의 기능을 모듈화하고 분리하여 유지보수성을 향상하고자 노력하였습니다.

Alamofire를 사용하여 네트워크 요청을 전송하고 응답을 처리
각 API 엔드포인트에 대한 URLRequest를 생성하고 관리
네트워크 요청은 APIClient를 통해 처리

APIRouter와 NetworkRouter 프로토콜을 도입하여 네트워크 요청의 엔드포인트와 매개변수를 정의하고 요청을 생성합니다. 이를 통해 요청의 구성이 단순화되고 유지보수성을 향상시키고자했습니다.

ChatService 클래스를 도입하여 채팅 서비스에 대한 요청을 처리합니다. 이 클래스는 APIClient 인스턴스를 의존성으로 받아와 사용합니다. 이를 통해 채팅 관련 로직이 분리하고자하였습니다.

🤔 조언을 얻고싶은 부분

Alamofire의 경우 Router패턴과 함께 사용되는 경우가 많은데, 따라서 네이밍을 관련된 이름으로 작성하였습니다.이 네이밍이 적절한지, 또 패턴을 잘 활용하고 있는지에 대해 궁금합니다.

현재 폴더 구조는 같은 역할에 따라 폴더를 나누었습니다. 폴더링은 수정하고싶은데 이부분에서 조언을 얻고싶습니다!

✨Feat/step1

ICS-Asan

안녕하세요 토미, 이지~
이번 스텝도 정말 고생많으셨습니다😄

여러분들이 조언을 부탁하신 부분에 대해 먼저 코멘트를 남기려고 합니다.

먼저 네이밍에 관련해서는 크게 어색하거나 잘못 사용되었다는 느낌을 주는 것들은 없었던 것 같습니다.
하나를 꼽아보자면 Network에 구현되어있는 파일들을 보면 어디서나 공통으로 사용할 수 있는 것들인 것 같은데 APIRouter는 ChatGPT API에 맞게 구현되어있는 것 같아요.
공통 로직과 ChatGPT API 관련 로직을 분리해보시면 좋을 것 같아요
조금 더 개선을 할 수 있는 부분을 말씀드리자면 아래 코멘트에도 남겨두었지만 프로토콜을 사용하는 이유가 조금 더 두드러지면 좋을 것 같아요.
프로토콜을 구현하고 채택하셨는데 해당 프로토콜 채택하지 않더라도 기능상으로 아무 문제가 없는 것 같아요.
프로토콜 사용에 대해 어떻게 하면 더 잘 활용할 수 있을지 고민해보시면 좋을 것 같아요.
추가로 조금 더 조언을 드리자면 역할을 분리하고 재사용하는 것을 고민하는 것은 좋지만 억지로 나눌 필요는 없을 것 같아요.
잘못 나누게되면 오히려 어떤 로직이 어디에 있었는지 찾기 어려워질 수 있고 하나의 기능을 수정하기 위해 여러파일들을 수정하게 되는 불편함이 있을 수도있어요.
언제나 그 적정선을 유지하기 위해 고민하면서 프로젝트를 진행하면 좋을 것 같아요!
(지금 문제가 있다는 것은 아닙니다!)

ICS-Asan · 2024-04-07T09:53:15Z

.gitignore

+## config ##
+*.xcconfig
+


현재 빌드가 정상적으로 되지 않아요.
혹시 이 파일을 gitignore에 추가하신 이유가 있으실까요??
여러분들의 로컬에 있는 프로젝트를 삭제한 이후에 다시 클론을 받아서 빌드를 진행해보시면 좋을 것 같아요.
gitignore에 추가한 파일이 어떤파일인지 어떤 역할을 하는 파일인지 알아보시면 좋을 것 같아요.
그리고 이 문제를 어떻게 해결할 수 있을지도 확인해보세요!

xcconfig란?

Xcode 프로젝트의 빌드 설정을 외부화하여 관리할 수 있게 해주는 파일입니다.

.xcconfig 확장자로 저장되며, 이를 사용하면 빌드 설정을 코드로 관리할 수 있어 복잡한 프로젝트에서 빌드 설정을 좀 더 쉽게 관리할 수 있습니다.

api key를 git에 노출시키지 않도록 하기 위해서 추가하였습니다.

의도는 이해했습니다.
그렇지만 git을 통해서 pull을 받았을때 정상 빌드가 되지 않고 xcconfig를 사용하지 않고 빌드하는 경우 api key가 없어 API요청에 실패하게 되어있습니다.
데이터의 보안에 관련해서는 어떻게 처리하면 좋을지 추후에 조금 더 알아보시고 이번에는 추가해주시면 좋을 것 같네요 ㅎㅎ

ICS-Asan · 2024-04-07T09:58:07Z

ChatBot/ChatBot.xcodeproj/project.pbxproj

-				IPHONEOS_DEPLOYMENT_TARGET = 15.0;
+				IPHONEOS_DEPLOYMENT_TARGET = 14.0;


어떤 기준으로 iOS14를 선택하게 되셨는지 궁금해요~
그리고 현재 프로젝트와 타켓의 최소지원 버전이 다르게 세팅되어있어요!

음.. 이상하게도 커밋에는 있지만 최소지원 버전을 14.0 로 바꾼 기억이 없네요.. 따라서 이유는 없습니다..! 😅
저번의 의견을 참고해서 카카오톡의 현재 최소지원 버전을 기준으로 프로젝트와 타겟의 최소지원 버전을 15.0 으로 통일하기로 했습니다.

ICS-Asan · 2024-04-07T10:04:05Z

ChatBot/ChatBot/Network/APIClient.swift

+final class APIClient {
+    static let shared = APIClient()


현재 APIClient의 역할이 Network요청을 보내는 것 이외에는 하고 있지 않아요.
APIClient를 싱글톤으로 구현하신 이유가 있으실까요??

APIClient를 싱글톤으로 구현한 이유 중 하나는 애플리케이션의 다양한 부분에서 동일한 APIClient 인스턴스에 접근할 수 있으므로 코드 중복을 줄이고 일관된 네트워킹 인터페이스를 유지할 수 있습니다. 또한, 싱글톤을 사용하여 코드를 간소화하고 객체 생성 및 관리에 대한 부담을 줄이기위해서 사용하였습니다.

여러분들의 의도라면 enum에 static func으로 구현해도 동일하게 동작합니다.
싱글톤 패턴과 static에 대해서 조금 더 구체적으로 고민해보시는게 좋을 것 같아요.

ICS-Asan · 2024-04-07T10:11:48Z

ChatBot/ChatBot/Network/APIClient.swift

+                        case 400...499:
+                            completion(.failure(.pathError))


현재 앱을 실행하면 Error: pathError가 로그에 찍히고 있어요.
왜 에러가 발생하는지 확인하고 해결해주세요!
그리고 400대 에러코드가 언제 발생할 수 있는지도 다시 고민해보시면 좋을 것 같아요.
경로가 잘못된 상황 이외에도 발생하는 경우가 있는지 확인해보세요!

아마 이전 API키로 사용하셔서 pathError가 발생하는것같아요 header에 api 키를 정확하게 입력하게된다면 pathError가 발생하지않을것같습니다.
400대 에러코드 종류

400 Bad Request: 클라이언트 요청이 서버에서 이해할 수 없는 형식으로 되어 있거나 잘못된 구문을 포함하고 있음

401 Unauthorized: 클라이언트가 인증되지 않았거나, 유효한 인증 정보가 부족하여 요청이 거부되었음을 의미하는 상태값

403 Forbidden: 서버가 해당 요청을 이해했지만, 권한이 없어 요청이 거부되었음을 의미하는 상태

404 Not Found: 클라이언트가 요청한 리소스를 서버에서 찾을 수 없음을 나타냅니다. 즉, 요청한 URL이 유효하지 않거나 해당 리소스가 서버에 존재하지 않는 경우입니다.

제가 말씀 드린 부분은 400대 에러가 발생하는 경우가 path가 잘못된 경우 말고도 여러분들이 말씀하신 api key가 잘못되는 경우도 있어요.
그리고 여러분들이 찾아주신 케이스 중에 권한이 없거나, 데이터를 잘못 전달했을때 등에서도 400대 에러가 발생할 수 있어요.
그런데 현재 여러분들이 구현하신 코드에서는 400대에서 발생하는 모든 에러가 pathError로 보이게 되어 명확한 원인을 알 수 없는 것 같아 코멘트를 남겼습니다.
지금은 message를 로그로 확인할 수 있도록 수정해주셔서 네이밍 정도만 조금 더 포괄적으로 수정되면 좋을 것 같아요.

p.s. 그리고 현재 NetworkError의 case중에 requestError는 사용하고 있지 않은 것 같아요!

ICS-Asan · 2024-04-07T10:18:15Z

ChatBot/ChatBot/Network/APIConstants.swift

+enum HTTPHeaderField: String {
+    case authentication = "Authorization"
+    case contentType = "Content-Type"
+}
+
+enum ContentType: String {
+    case json = "application/json"
+}


HTTPHeaderField, ContentType도 사용하는 방법을 보니 API요청시 필요한 기본 세팅 문자열을 다루고 있네요.
APIConstants에 포함되어도 될 것 같은데 별도로 분리해서 선언한 이유가 궁금합니다.
그리고 여러분들의 class, struct, enum 선택 기준도 궁금합니다!

API 요청에서 반복적으로 사용되는 HTTP 헤더 필드와 콘텐츠 타입을 정의함으로써 모듈화와 코드의 가독성을 높였습니다. 하드코딩하는 대신 의미 있는 이름을 사용할 수 있고, API 요청에 필요한 기본 세팅이 변경될 경우, 해당 세팅을 열거형의 값만 수정하여 유지보수를 쉽게 할 수 있습니다.

애플에서는 기본적으로 특별한 이유가 없다면 struct를 사용하는것을 권장하는 것으로 알고 있습니다.
그 이유라고 하면 다음과 같은 기준이 있을 것입니다.

해당 객체가 값으로 쓰일 것인지 래퍼런스 타입으로 쓰일것인지 (불변성과 가변성)

대입 되는 경우가 생성 되는 경우보다 많을 시 class

하지만 비교적 많은 데이터, 메모리 용량이 큰 타입일 경우 class 유리할 때도 있다

열거형도 값 타입으로서 struct를 사용 할 때와 같은 기준으로 사용하지만 연관된 값들을 그룹화하여 표현하는 데 유리한 점이 있기 때문에 선택지를 제한하거나 관련된 값들을 묶어서 사용할 때 선택합니다.

먼저 공통으로 사용되는 값들을 이렇게 분리해서 별도로 선언하여 재사용할 수 있게 구현하신 것은 좋은 방법이라고 생각합니다.
다만 제가 코멘트를 남겨놓았던 이유가 몇가지 있는데요.

APIConstants, HTTPHeaderField, ContentType 사용 방법이 동일한 것 같은데 APIConstants는 struct에 static으로 프로퍼티를 선언해서 사용하고 있고 나머지 두개는 enum으로 구현해서 rawValue를 사용하고 있어 같은 목적과 사용 방법이 같은데 구현이 달라서 코멘트 남겼습니다.

HTTPHeaderField, ContentType에 있는 값들을 포괄적으로 보면 APIConstants의 내용에도 포함될 수 있는 것 같아 별도로 분리하신 이유를 여쭤봤습니다.

baseURL를 static으로 선언해두었는데 APIConstants 인스턴스를 생성하는 경우가 있을지?도 고민해보시면 좋을것 같아 질문드렸습니다.

ICS-Asan · 2024-04-07T10:30:00Z

ChatBot/ChatBot/Network/APIRouter.swift

+
+import Alamofire
+
+enum APIRouter: NetworkRouter {


APIRouter가 반드시 가져야하는 프로퍼티를 정의한다는 측면에서는 NetworkRouter 프로토콜을 구현하고 채택하는 것이 좋아 보입니다.
다만 해당 기능 이외에는 프로토콜을 채택할 이유가 조금 부족해보여요.
현재는 APIRouter가 NetworkRouter 프로토콜을 채택하지 않더라고 기능에 아무 문제가 없어요
프로토콜을 타입으로 활용하거나 기본구현을 통해 공통 로직을 분리하는 등의 방법으로 조금 더 프로토콜을 사용하는 이점들을 가져가는 방향으로 고민을 해보시면 좋을 것 같습니다.
다른 사람들은 프로토콜의 어떤 장점을 어떻게 활용하고 있는지도 찾아보면 도움이 될 것 같네요!

프로토콜의 기본 구현을 통해 공통 로직을 분리했습니다!!

ICS-Asan · 2024-04-07T10:31:25Z

ChatBot/ChatBot/Service/ChatService.swift

+protocol ChatServiceProtocol {
+    func sendChatRequest(message: String, completion: @escaping (NetworkResult<ResponseDTO>) -> Void)
+}
+
+final class ChatService: ChatServiceProtocol {


이 부분도 위 코멘트랑 비슷한 것 같아요.
ChatServiceProtocol 구현하고 채택하신 이유가 있으신가요??
해당 프로토콜을 사용하는 곳이 없는 것 같아 질문드려요!

ChatService를 ChatServiceProtocol로 정의함으로써 해당 클래스가 특정 프로토콜을 준수함을 명시적으로 나타내었습니다. 이렇게 하면 ChatService가 특정 프로토콜을 따르는 것을 보장하고, 이 프로토콜에서 정의된 메서드를 구현해야 한다는 것을 알 수 있습니다. 물론 현재 ChatServiceProtocol을 사용하는 곳이 없지만 ChatService를 사용하는 클래스에서 ChatServiceProtocol을 통해 의존성을 주입을 생각할 수 있기 때문에 확장성과 유연성을 고려하였습니다.

ICS-Asan · 2024-04-07T10:33:13Z

ChatBot/ChatBot/Util/AppConfig.swift

+        guard let key = Bundle.main.object(forInfoDictionaryKey: "API_KEY") as? String else {
+            assertionFailure("API_KEY를 찾을 수 없음")
+            return ""
+        }
+        return key
+    }


assertionFailure가 실행되면 어떻게 되는지 직접 확인해보셨을까요??
어떤 이유로 assertionFailure를 사용하셨는지 궁금합니다.

assertionFailure 함수가 실행되면, 해당 코드 블록에서 프로그램이 중단됩니다. 이는 애플리케이션의 실행이 멈추고, 개발자에게 오류 메시지와 함께 디버깅을 위한 정보를 제공합니다.

기본적으로 assertionFailure는 디버그 빌드에서만 동작하며, 릴리스 빌드에서는 무시됩니다. 따라서 개발자가 개발 중에만 논리적 오류를 파악하고 해결할 수 있도록 도와줍니다. 개발 중 디버깅 목적으로만 사용되어야 하며, 릴리스 빌드에서는 오류 처리를 위한 적절한 방법으로 대체되어야 합니다.

따라서 이러한 이유로 사용하였습니다.

ICS-Asan · 2024-04-07T10:34:14Z

ChatBot/ChatBot/Util/Extensions/MessageDTO+DictionaryRepresentation.swift

+extension MessageDTO {
+    func dictionaryRepresentation() -> [String: Any] {
+        return [
+            "role": role.rawValue,
+            "content": content
+        ]
+    }
+}


해당 기능은 MessageDTO파일에 두지 않고 별도의 파일로 분리하신 이유가 궁금합니다.

일반적으로 코드를 작성할 때, 한 파일에 모든 코드를 작성하는 것보다 각 기능이나 역할에 따라 파일을 분리하여 관리하는 것이 유지보수할 때 좋다고 생각했습니다. 또한, dictionaryRepresentation() 메서드는 해당 DTO를 딕셔너리로 변환하는 공통 기능을 제공한다고 판단하여 구조를 보다 명확하게 하기 위해서 분리하였습니다!

그렇다면 프로토콜을 활용해보는 것은 어떨까요?
파일이 분리되어있어서 이 파일의 존재를 파악하기 전까지는 MessageDTO, RequestDTO가 Dictionary타입으로 변환하는 기능을 제공한다는 것을 인지하기 어려울 것 같아요.
MessageDTO, RequestDTO가 공통으로 제공하는 기능에 대한 프로토콜을 채택하고 있다면 이 파일의 존재를 모르더라도 Dictionary타입으로 변환하는 기능을 제공할 것이라는 것을 빠르게 인지할 수 있을 것 같아요.

ICS-Asan · 2024-04-07T10:34:25Z

ChatBot/ChatBot/Util/Extensions/RequestDTO+DictionaryRepresentation.swift

+extension RequestDTO {
+    func dictionaryRepresentation() -> [String: Any] {
+        var parameters: [String: Any] = [:]
+
+        parameters["model"] = model.rawValue
+        parameters["stream"] = stream
+        parameters["messages"] = messages.map { $0.dictionaryRepresentation() }
+
+        return parameters
+    }


위 코멘트와 동일한 질문입니다!

yoruck2 and others added 9 commits April 2, 2024 14:33

Merge pull request #1 from yoruck2/feat/STEP1

9e888d0

✨Feat/step1

add: api key config에 추가

d526d02

add: plist에 api key 추가

70c7a81

feat: 네트워크 레이어 작업 구현

7a1f1ad

feat: dictionary 타입 변환 메서드 생성

da4153d

feat: 네트워크 요청 APIClient 구현

1fcfe85

feat: ChatService 생성

c48cf43

feat: 요청 테스트

4b7736b

refactor: .gitignore 설정 재적용

1ab9a9c

ICS-Asan reviewed Apr 7, 2024

View reviewed changes

angryeon7 added 6 commits April 9, 2024 19:56

chore: 최소지원 버전 수정

e703a3d

refactor: APIRouter 공통 로직을 분리

cdd02f7

refactor: 구체적인 에러메세지 출력

f739150

feat: Dictionary타입 변환 프로토콜 생성

ac01d9e

chore: 불필요한 코드 수정

b4a5bdc

refactor: APIClient enum으로 구현

d58b3f2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AI 챗봇 앱 [STEP 2] 토미, 이지 #42

AI 챗봇 앱 [STEP 2] 토미, 이지 #42

angryeon7 commented Apr 4, 2024

ICS-Asan left a comment

ICS-Asan Apr 7, 2024

angryeon7 Apr 8, 2024 •

edited

Loading

ICS-Asan Apr 10, 2024

ICS-Asan Apr 7, 2024

yoruck2 Apr 9, 2024 •

edited

Loading

ICS-Asan Apr 7, 2024

angryeon7 Apr 9, 2024

ICS-Asan Apr 10, 2024

ICS-Asan Apr 7, 2024

angryeon7 Apr 9, 2024

ICS-Asan Apr 10, 2024

ICS-Asan Apr 7, 2024

angryeon7 Apr 9, 2024

yoruck2 Apr 9, 2024

ICS-Asan Apr 10, 2024

ICS-Asan Apr 7, 2024

angryeon7 Apr 9, 2024

ICS-Asan Apr 7, 2024

angryeon7 Apr 9, 2024

ICS-Asan Apr 7, 2024

angryeon7 Apr 9, 2024

ICS-Asan Apr 7, 2024

angryeon7 Apr 8, 2024

ICS-Asan Apr 10, 2024

ICS-Asan Apr 7, 2024

		IPHONEOS_DEPLOYMENT_TARGET = 15.0;
		IPHONEOS_DEPLOYMENT_TARGET = 14.0;

		## config ##
		*.xcconfig

AI 챗봇 앱 [STEP 2] 토미, 이지 #42

Are you sure you want to change the base?

AI 챗봇 앱 [STEP 2] 토미, 이지 #42

Conversation

angryeon7 commented Apr 4, 2024

🔍 What is this PR?

네트워크 통신 개선

🤔 조언을 얻고싶은 부분

ICS-Asan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angryeon7 Apr 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoruck2 Apr 9, 2024 • 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

Choose a reason for hiding this comment

angryeon7 Apr 8, 2024 •

edited

Loading

yoruck2 Apr 9, 2024 •

edited

Loading