🔗 :: (#275) 반 별 취업 현황 API 연동 #276

leejh08 · 2025-03-04T02:14:16Z

개요

반 별 취업 현황 API 구현

작업사항

반 별 취업 현황 API 구현

UI

cyj513 · 2025-03-04T02:14:53Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

Projects/Presentation/Sources/Home/EmployStatus/EmployStatusViewController.swift

Projects/Presentation/Sources/Home/EmployStatus/ClassEmploymentViewController.swift

cyj513 · 2025-03-04T05:18:09Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

cyj513 · 2025-03-04T05:23:34Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

Projects/Presentation/Sources/Home/EmployStatus/Components/ClassTitle.swift

juyeong525 · 2025-03-04T05:39:38Z

Projects/Presentation/Sources/Home/EmployStatus/ClassEmploymentViewModel.swift

+        let classInfo = input.viewWillAppear
+            .flatMapLatest { [fetchEmploymentStatusUseCase] in
+                fetchEmploymentStatusUseCase.execute()
+            }
+            .map { applications in
+                applications.first { $0.classId == self.classNumber } ?? ApplicationEntity(
+                    applicationID: 0,
+                    recruitmentID: 0,
+                    company: "",
+                    companyLogoUrl: "",
+                    attachments: [],
+                    applicationStatus: .approved,
+                    classId: self.classNumber,
+                    employmentRateResponseList: [],
+                    totalStudents: 0,
+                    passedStudents: 0
+                )
+            }
+            .share(replay: 1)


@leejh08 share(replay: 1)를 사용해 처리한 이유가 있을까요?

classinfo를 구독할때 중복 요청이 생길거같아 써봤습니다

share 은 여러곳에서 subscribe을 했을경우 여러 스트림이 생겨 중복 호출될때 단일 스트림으로 만들어 주기 위해 사용합니다.
하지만 현재 코드 상으로는 ViewController에 bind 부분에서 output를 subscribe를 하여 중복 스트림이 생기지 않을 것으로 보입니다.
써봤다보다는 쓴 이유가 명확해야 한다고 생각합니다.

그래도 중복호출을 생각하고 share로 미리 방지 하는건 좋은거 같아요👍🏿

cyj513 · 2025-03-04T10:47:48Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

cyj513 · 2025-03-04T12:24:53Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

Projects/Data/Sources/DTO/Applications/ClassEmploymentListResponseDTO.swift

Projects/Domain/Sources/Entities/Applications/ApplicationEntity.swift

Projects/Flow/Sources/Home/EmployStatusFlow.swift

Projects/Presentation/Sources/Home/EmployStatus/ClassEmploymentViewController.swift

HongSJae · 2025-03-05T05:18:49Z

Projects/Presentation/Sources/Home/EmployStatus/ClassEmploymentViewModel.swift

+            .map { applications in
+                applications.first { $0.classId == self.classNumber } ?? ApplicationEntity(
+                    applicationID: 0,
+                    recruitmentID: 0,
+                    company: "",
+                    companyLogoUrl: "",
+                    attachments: [],
+                    applicationStatus: .approved,
+                    classId: self.classNumber,
+                    employmentRateResponseList: [],
+                    totalStudents: 0,
+                    passedStudents: 0
+                )
+            }


리뷰 남겨드린 내용 토대로 ApplicationEntity 수정 및 추가 하시면 개선이 필요한 부분 같습니다.

코드가 너무 지저분한 거 같아요 중복작업 이 많아요

Projects/Presentation/Sources/Home/EmployStatus/EmployStatusViewController.swift

HongSJae · 2025-03-05T05:25:54Z

Projects/Presentation/Sources/Home/EmployStatus/EmployStatusViewController.swift

+            .forEach { classNumber, button in
+                button.rx.tap
+                    .map { classNumber + 1 }
+                    .bind(to: classButtonTapped)
+                    .disposed(by: disposeBag)
+            }


classNumber가 아니라 index라고 하는게 맞는 것 같습니다

아니면 굳이 enumerated()를 쓰지 말고 foreach에서 button에 접근 할 수 있는데
ClassButton에 상수를 만들어 classNumber를 가져올 수 있게 하면 좋을 것 같아요

아니면 굳이 enumerated()를 쓰지 말고 foreach에서 button에 접근 할 수 있는데 ClassButton에 상수를 만들어 classNumber를 가져올 수 있게 하면 좋을 것 같아요

선배님이 말씀하신 방식으로 처음 코드를 작성하였는데 주영 선배님이 추천하는 방식으로 바꾼게 이 코드입니다
혹시 어떤 방식이 조금 더 효율적일까요?

음 엄연히 다른 코드라고 생각해요. 기존에 튜플로 매핑해둔 구문을 enumerate를 사용해 더 깔끔한 코드를 제안했어요
저는 Projects/Presentation/Sources/Home/EmployStatus/Components/ClassButton.swift #line=25 에 init으로 이미 classNumber를 받고 있으니 상수로 classNumber을 하나 갖고, 잇으면

Suggested change

.forEach { classNumber, button in

button.rx.tap

.map { classNumber + 1 }

.bind(to: classButtonTapped)

.disposed(by: disposeBag)

}

.forEach { button in

button.rx.tap

.map { button.classNumber }

.bind(to: classButtonTapped)

.disposed(by: disposeBag)

}

이런식으로 굳이 index(offset)을 사용하지 않고도 활용할 수 있어요. 이것도 가독성 차원에서 약간 의견을 낸 거 뿐이긴 하니 뭐가 더 나을지는 개인이 판단해보세요!

추가로 tag를 써서 mapping 하라는거랑도 거의 비슷한 내용입니다~ 한번 생각해봐요!

cyj513 · 2025-03-05T11:18:01Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

Projects/Presentation/Sources/Home/EmployStatus/ClassEmploymentViewController.swift

cyj513 · 2025-03-06T07:04:36Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

cyj513 · 2025-03-07T00:34:39Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

cyj513 · 2025-03-07T05:59:51Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

cyj513 · 2025-03-09T12:44:49Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

cyj513 · 2025-03-09T13:13:43Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

cyj513 · 2025-03-09T13:22:53Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

cyj513 · 2025-03-09T13:24:42Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

HongSJae · 2025-03-11T05:58:03Z

Projects/Presentation/Sources/Home/EmployStatus/ClassEmploymentViewController.swift

+    private func updateCompanyList(_ info: EmploymentEntity) {
+        let allCompanies = info.employmentRateResponseList + Array(
+            repeating: EmploymentCompany.empty,
+            count: max(0, 16 - info.employmentRateResponseList.count)


의견입니다만은, empty를 제가 활용해보라 했지만.. 그것두 더미 데이터니까 그냥 nil을 넣는 방법도 있다는거 알아두면 좋아요~

선배님께서 남겨주신 의견이 더 좋은 코드라고 생각되어, nil을 활용하여 수정해보았습니다! 😊

HongSJae · 2025-03-11T05:58:49Z

Projects/Domain/Sources/Entities/Applications/EmploymentEntity.swift

+    public func with(classID: Int) -> EmploymentEntity {
+        EmploymentEntity(
+            classID: classID,
+            employmentRateResponseList: self.employmentRateResponseList,
+            totalStudents: self.totalStudents,
+            passedStudents: self.passedStudents
+        )
+    }


어디서 활용되는걸까요?

먼저 ClassEmploymentViewModel에서 with 메서드를 활용하였습니다.
employments.first { $0.classID == classNumber } ?? EmploymentEntity.empty.with(classID: classNumber)
추가적으로 설명 드리자면
모든 반의 취업 api를 가져온 후 정보를 보려는 반의 데이터를 찾습니다.
만약 없다면 empty 객체를 생성하고 classID만 해당 반 번호로 변경하여 반환합니다.
이는 데이터가 없는 경우에도 해당 반의 빈 정보를 표시하기 위하여 작업했습니다

cyj513 · 2025-03-12T03:31:01Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

cyj513 · 2025-03-12T13:42:14Z

All File Checked ✅

✅	Auto Pull Request SwiftLint Success!!!!

leejh08 added 7 commits March 2, 2025 01:03

🧩 :: application 데이터 처리

9783713

🧩 :: DI 추가

30789db

🧩 :: flow 수정

77d3eff

🧩 :: dto, entity, usecase 추가

48a5c4b

🧩 :: foreach로 수정

567b2a8

🧩 :: classCollectionCell 추가

892347e

🧩 :: 반별 취업 현황추가

4d88e98

leejh08 added the ✨feat 새로운 기능을 추가 할 경우 label Mar 4, 2025

leejh08 requested review from HongSJae, juyeong525, cyj513, ray3238 and circle0802 March 4, 2025 02:14

leejh08 self-assigned this Mar 4, 2025

leejh08 linked an issue Mar 4, 2025 that may be closed by this pull request

반 별 취업 현황 API 연동 #275

Open

leejh08 changed the title ~~🔗 :: (#275) 반 별 취업 현황 API~~ 🔗 :: (#275) 반 별 취업 현황 API 연동 Mar 4, 2025

juyeong525 reviewed Mar 4, 2025

View reviewed changes

Projects/Presentation/Sources/Home/EmployStatus/EmployStatusViewController.swift Outdated Show resolved Hide resolved

Projects/Presentation/Sources/Home/EmployStatus/ClassEmploymentViewController.swift Outdated Show resolved Hide resolved

🧩 :: classButton 수정

d0b7249

🧩 :: 각 반 enum 처리

4cdd154

juyeong525 reviewed Mar 4, 2025

View reviewed changes

🛠️ :: enum 네이밍

e0597d4

🛠️ :: enum에 맞게 수정

768a563

HongSJae requested changes Mar 5, 2025

View reviewed changes

🧩 :: configureNavigation으로 수정

f98dbd8

juyeong525 reviewed Mar 6, 2025

View reviewed changes

Projects/Presentation/Sources/Home/EmployStatus/ClassEmploymentViewController.swift Outdated Show resolved Hide resolved

🧩 :: upperCase로 수정

7860ff3

🧩 :: 스위치문 수정

c17b8ec

🧩 :: 반별 버튼 이름 변경

ec708d8

🧩 :: EmploymentEntity 추가

081b6da

🧩 :: drive로 수정

a2a18e7

🧩 :: 불필요한 코드 수정

cbd666a

🧩 :: toDomain 처리

b52e0ed

HongSJae approved these changes Mar 11, 2025

View reviewed changes

🧩 :: 더미데이터에서 nil로 변경

caf0be4

🧩 :: 차트 뷰 스타일 수정

3057915

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🔗 :: (#275) 반 별 취업 현황 API 연동 #276

🔗 :: (#275) 반 별 취업 현황 API 연동 #276

leejh08 commented Mar 4, 2025

cyj513 commented Mar 4, 2025

cyj513 commented Mar 4, 2025

cyj513 commented Mar 4, 2025

juyeong525 Mar 4, 2025

leejh08 Mar 4, 2025

juyeong525 Mar 6, 2025 •

edited

Loading

juyeong525 Mar 6, 2025

cyj513 commented Mar 4, 2025

cyj513 commented Mar 4, 2025

HongSJae Mar 5, 2025

HongSJae Mar 5, 2025

HongSJae Mar 5, 2025

HongSJae Mar 5, 2025

leejh08 Mar 6, 2025

HongSJae Mar 6, 2025

cyj513 commented Mar 5, 2025

cyj513 commented Mar 6, 2025

cyj513 commented Mar 7, 2025

cyj513 commented Mar 7, 2025

cyj513 commented Mar 9, 2025

cyj513 commented Mar 9, 2025

cyj513 commented Mar 9, 2025

cyj513 commented Mar 9, 2025

HongSJae Mar 11, 2025

leejh08 Mar 12, 2025

HongSJae Mar 11, 2025

leejh08 Mar 12, 2025

cyj513 commented Mar 12, 2025

cyj513 commented Mar 12, 2025

🔗 :: (#275) 반 별 취업 현황 API 연동 #276

Are you sure you want to change the base?

🔗 :: (#275) 반 별 취업 현황 API 연동 #276

Conversation

leejh08 commented Mar 4, 2025

개요

작업사항

UI

cyj513 commented Mar 4, 2025

All File Checked ✅

cyj513 commented Mar 4, 2025

All File Checked ✅

cyj513 commented Mar 4, 2025

All File Checked ✅

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juyeong525 Mar 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyj513 commented Mar 4, 2025

All File Checked ✅

cyj513 commented Mar 4, 2025

All File Checked ✅

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

cyj513 commented Mar 5, 2025

All File Checked ✅

cyj513 commented Mar 6, 2025

All File Checked ✅

cyj513 commented Mar 7, 2025

All File Checked ✅

cyj513 commented Mar 7, 2025

All File Checked ✅

cyj513 commented Mar 9, 2025

All File Checked ✅

cyj513 commented Mar 9, 2025

All File Checked ✅

cyj513 commented Mar 9, 2025

All File Checked ✅

cyj513 commented Mar 9, 2025

All File Checked ✅

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

cyj513 commented Mar 12, 2025

All File Checked ✅

cyj513 commented Mar 12, 2025

All File Checked ✅

juyeong525 Mar 6, 2025 •

edited

Loading