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

[개발자 비상근무] 짱구 미션 제출합니다. #18

Open
wants to merge 8 commits into
base: 02ggang9-main
Choose a base branch
from

Conversation

02ggang9
Copy link
Collaborator

@02ggang9 02ggang9 commented Mar 8, 2024

구현하면서 고민되었던 점

다른 분들은 어떻게 비상 근무자 배정 규칙을 구현하셨는지 궁금합니다..!!

제가 구현 능력이 너무 떨어져서 시간이 많이 걸렸습니다..ㅠ 이번에 합격하신 분들 대단합니다 bb

자랑스러운 점

없습니다..!! 이번 미션은 구조보다는 구현쪽인 것 같아 힘들었습니다..ㅠ

@02ggang9 02ggang9 requested review from KIJUNG-CHAE and 1jeongg March 8, 2024 04:04
@02ggang9 02ggang9 self-assigned this Mar 8, 2024
Copy link
Collaborator

@1jeongg 1jeongg left a comment

Choose a reason for hiding this comment

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

이번주도 고생하셨어요!! 제가 설명 능력이 부족해서 이해하셨을지 모르겠지만 제가 구현한 방법도 적어놨어요!

바쁘실텐데 대단하십니다!! 앞으로도 화이팅입니다 🥰🥰

Comment on lines +21 to +27
val month = userInput[0]
.toIntOrNull()
?.takeIf { it in 1..12 }
?: throw IllegalArgumentException(InputView.INVALID_INPUT_VALUE)

val day = Day.entries.firstOrNull{ it.day == userInput[1] }
?:throw IllegalArgumentException(InputView.INVALID_INPUT_VALUE)
Copy link
Collaborator

Choose a reason for hiding this comment

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

, 를 포함하지 않아서 split 되지 않는다면 userInput에 접근할 때 에러가 발생하지 않을까요?

{ Console.readLine() }
)

val weekendEmployee = getEmergencyWorker(
Copy link
Collaborator

Choose a reason for hiding this comment

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

평일 근무자와 휴일 근무자가 동일한지 체크하는 로직도 필요할 것 같습니다!

SUNDAY("일", true),
;

fun next(): Day = entries[(this.ordinal + 1) % entries.size]
Copy link
Collaborator

Choose a reason for hiding this comment

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

오.. 이렇게 쓸 수 있군요!! 깔끔하고 좋습니다 👍👍

Choose a reason for hiding this comment

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

저도 배워갑니다! 🙂

Comment on lines +3 to +14
class EmergencySchedule(
val month: Month,
val dayOfMonth: Int,
val day: Day,
val isHoliday: Boolean,
val name: String,
) {

override fun toString(): String {
return "EmergencySchedule(month=$month, dayOfMonth=$dayOfMonth, day=$day, isHoliday=$isHoliday, name='$name')"
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

데이터만 저장하는 클래스는 data class로 정의하는 건 어떨까요?

data class로 사용하면 toString, equals, hashCode 와 같은 메서드를 정의해주고 정확성과 일관성을 한 번더 검사해주기 때문에 좋을 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

감사합니다~!! 수정하도록 하겠습니다!!

) {

fun generateEmergencySchedule(): List<EmergencySchedule> {
var holidayIndex = 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

오 문제 조건대로 겹치면 swap 하는 방식으로 구현하셨군요!

저도 이 방식으로 풀다가 변수를 4개 이상 선언하다가 마음에 안 들어서 다른 방식으로 구현했어요!


저는 평일 근무자와 휴일 근무자를 전체 날짜만큼 순서대로 만들어두고 전체 근무자 리스트의 마지막 사람과 이름이 다른 가장 앞에있는 근무자 를 전체 근무자 리스트에 추가하고 평일/휴일 근무자 리스트에서 해당 근무자를 삭제하는 방법을 사용했어요!

예를 들어 입력이 1,월, 가,나,다,라,마, 가,나,다,마,라 이렇게 주어진다면 이렇게 만들어두고 휴일이다 -> 그러면 holiday 근무자 중에서 전체 workerList의 마지막 근무자랑 이름이 안 겹치는 사람을 workList에 추가해줬어요!

  • weekdayWorker = [가,나,다,라,마,가,나,다,...]
  • holidayWorker = [가,나,다,마,라,가,나,다,...]
  • workList 에는 가(휴일)이 먼저 들어가고 다음엔 와 다른 가장 첫 원소인 를 넣어주는 거죠!

제가 설명을 잘 못해서 ㅠㅠ 뭔가 잘못된 부분 있거나 이해 안되시면 말씀해주세요!!

private val holidayWorker = MutableList(endDate) { i -> oncallInformationDTO.holidayWorkerList[i%size] }
private val weekdayWorker = MutableList(endDate) { i -> oncallInformationDTO.weekdayWorkerList[i%size] }

fun getWorkerList(): List<String> {
    val workerList = mutableListOf<String>()
    for (date in 1..endDate) {
        val workers = getWorkerList(date)
        val workerIndex = workers.indexOfFirst { workerList.isEmpty() || it != workerList.last() }
        workerList.add(workers[workerIndex])
        workers.removeAt(workerIndex)
    }
    return workerList.toList()
}
private fun getWorkerList(date: Int) = if (isHoliday(date)) holidayWorker else weekdayWorker

Copy link
Collaborator

Choose a reason for hiding this comment

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

좋네요! 👍👍 변수가 확실히 줄어들고 index를 유지할 필요도 없어서 훨씬 직관적이네요 ㅎㅎ

굳이 굳이 단점을 꼽자면 removeAt 할 때 맨 앞 원소 or 바로 뒤 원소를 지우기 때문에 모든 원소가 한 칸 씩 앞당겨지면서 성능 면에선 조금 불편 해 보일 수 있겠네요. 근데 workerList 원소 최대 개수가 31개라 굳이 가독성을 포기하면서 성능을 챙길 필요는 없어 보입니다. 👍👍

SEPTEMBER(9, 30),
OCTOBER(10, 31),
NOVEMBER(11, 30),
DECEMBER(12, 31)
Copy link
Collaborator

Choose a reason for hiding this comment

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

콤마 빠졌어요!!!!

Copy link
Collaborator

Choose a reason for hiding this comment

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

JDK 표준 라이브러리 Month, Day 사용해보셔도 좋을 것 같아요~

import kotlin.reflect.KParameter
import kotlin.reflect.cast

object Container {
Copy link
Collaborator

Choose a reason for hiding this comment

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

뭔지 모르겠지만 멋있습니다 ~~!

Copy link
Collaborator

@gusah009 gusah009 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~ 👍👍 컨테이너도 만들어보시고 많은 시간을 쓰신게 보이네요! 고생하셨습니다~~

import kotlin.reflect.KParameter
import kotlin.reflect.cast

object Container {
Copy link
Collaborator

Choose a reason for hiding this comment

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

simple Container 를 만들어버리셨군요 ㅋㅋㅋㅋ 👍

) {

fun generateEmergencySchedule(): List<EmergencySchedule> {
var holidayIndex = 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

좋네요! 👍👍 변수가 확실히 줄어들고 index를 유지할 필요도 없어서 훨씬 직관적이네요 ㅎㅎ

굳이 굳이 단점을 꼽자면 removeAt 할 때 맨 앞 원소 or 바로 뒤 원소를 지우기 때문에 모든 원소가 한 칸 씩 앞당겨지면서 성능 면에선 조금 불편 해 보일 수 있겠네요. 근데 workerList 원소 최대 개수가 31개라 굳이 가독성을 포기하면서 성능을 챙길 필요는 없어 보입니다. 👍👍

Comment on lines +23 to +31
if (isHoliday || currentDayOfMonth.isWeekend) {
swapIfSameEmployee(weekdayEmployee, holidayEmployee, weekdayIndex, holidayIndex)
schedule.add(EmergencySchedule(currentMonth, dayCounter, currentDayOfMonth, isHoliday, holidayEmployee[holidayIndex]))
holidayIndex = incrementIndex(holidayIndex, holidayEmployee, originalHolidayEmployee)
} else {
swapIfSameEmployee(holidayEmployee, weekdayEmployee, holidayIndex, weekdayIndex)
schedule.add(EmergencySchedule(currentMonth, dayCounter, currentDayOfMonth, isHoliday, weekdayEmployee[weekdayIndex]))
weekdayIndex = incrementIndex(weekdayIndex, weekdayEmployee, originalWeekdayEmployee)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

if 와 else 코드가 비슷해보이네요. 잘 묶어서 중복코드를 제거하면 더 좋을 것 같습니다~ 👍

Comment on lines +11 to +13
override fun toString(): String {
return "EmergencySchedule(month=$month, dayOfMonth=$dayOfMonth, day=$day, isHoliday=$isHoliday, name='$name')"
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

좋네요 👍👍

SEPTEMBER(9, 30),
OCTOBER(10, 31),
NOVEMBER(11, 30),
DECEMBER(12, 31)
Copy link
Collaborator

Choose a reason for hiding this comment

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

JDK 표준 라이브러리 Month, Day 사용해보셔도 좋을 것 같아요~

Copy link

@KIJUNG-CHAE KIJUNG-CHAE left a comment

Choose a reason for hiding this comment

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

리뷰가 늦어서 죄송합니다..!!
이미 리팩토링이나 재설계하셨을까봐 조심스럽네요 😨
마지막까지 너무 수고하셨습니다!!!

}
}

fun getEmergencyWorker(printMessage: () -> Unit, console: () -> String): MutableList<String> {

Choose a reason for hiding this comment

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

while try catch 문을 함수로 분리해서 중복을 줄여도 좋을 것 같습니다!

import kotlin.reflect.KParameter
import kotlin.reflect.cast

object Container {

Choose a reason for hiding this comment

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

컨테이너 구현 대단하십니다 👍🏻

}

private fun isHoliday(month: Month, day: Int): Boolean =
StatutoryHoliday.entries.any { it.month == month.month && it.day == day }

Choose a reason for hiding this comment

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

휴일을 판별하는 목적상 StatutoryHoliday클래스에 포함되어도 좋을 것 같습니다!

SUNDAY("일", true),
;

fun next(): Day = entries[(this.ordinal + 1) % entries.size]

Choose a reason for hiding this comment

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

저도 배워갑니다! 🙂

class EmergencyWork(
val emergencyWorkDate: Pair<Month, Day>,
var weekdayEmployee: MutableList<String>,
var holidayEmployee: MutableList<String>

Choose a reason for hiding this comment

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

mutableList를 var로 선언하신 이유가 있을까요?
리스트의 원소를 바꾸는 작업만있고 참조자체를 바꾸는 코드를 못찾았네요..!


require(userInput.all { it.length < 6 }) { InputView.INVALID_INPUT_VALUE }
require(userInput.distinct().size == userInput.size) { InputView.INVALID_INPUT_VALUE }
require(userInput.size in 5..35) { InputView.INVALID_INPUT_VALUE }

Choose a reason for hiding this comment

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

닉네임 길이, 비상근무 인원수를 상수로 관리하셔도 좋을 것 같아요!

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.

4 participants