[로또 미션] 김의천 미션 제출합니다.

[wzrabbit]

안녕하세요 리뷰어님 👋🏻 로또 미션을 제출합니다. 잘 부탁드립니다!
기능 자체는 많이 들어있지 않은 것 같은데 엄밀하게 코드를 짜니 어마무시하게 거대한 코드가 되어버리는 것 같군요

이번 미션에서 신경써본 점 ✅

• 데이터를 관리하는 대부분의 클래스들을 일급 컬렉션 으로 구현하였습니다. 일급 컬렉션으로 구현하면서 특히 신경 쓴 것은 데이터 검증 인데, 각 검증 로직들을 각 클래스 내에 두었습니다.
• 상수들을 enum 클래스로 관리하였습니다.
• for 문의 사용을 줄이고, stream, forEach 등을 사용해 코드의 의도를 나타내었습니다. stream 이 장난아니게 복잡하더군요.

일급 컬렉션 사용 소감

상당히 좋았어요!

미션 진행 전 구글링을 하면서 클래스들을 개발자분들이 어떻게 다루나 보았는데 신기하게도 도메인 로직을 담당하는 클래스 내부 자체에 검증 로직을 많이들 두었더라고요. 그 때는 그런갑다 하고 넘어갔는데 이렇게 직접 검증 로직들을 그 클래스에 넣고 관리해주니 코드의 신뢰도가 많이 올라갔다는 것을 느꼈습니다.

• 원시 값은 너무나도 잘못 조작되기 쉬운데, 이렇게 일급 컬렉션을 사용해 각 데이터를 클래스 안에 두어 사용하고, 그 과정에서 검증을 거치기에 이 인스턴스들이 들고 있는 값들은 올바른 값을 들고 있는 것이 보장 되는 것이 정말 좋더라고요.
• 사용하는 입장에서 검증에 대해 신경쓰지 않아도 된다는 점도 좋았어요. 이전 미션처럼 입력받을 때 한 번만 검증했다면 검증 로직을 분리해도 전혀 상관이 없었으나 이번 미션에서는 은근 같은 로직을 검증해야 하는 일이 많았어요. 예를 들어 로또 번호와 당첨 로또 번호는 목적만 다를 뿐 근본적인 구조와 검증해야 하는 로직이 같아요. 이 경우 검증 로직을 적는 것을 까먹거나 매번 검증 로직을 적어 가독성과 생산성이 떨어지는 문제가 예상되었는데 이 단점들이 싹 다 사라지는 느낌이었어요.
• 꼭 사용자의 입력이 아니더라도, 개발자의 구현 실수로 인해 잘못된 값이 일급 컬렉션에 넘어가더라도 이를 인지하고 예외를 발생시키니 실수를 줄이기에도 좋다는 생각이 들었어요.

결론적으로 데이터를 관리하기에 안전한 방식처럼 느껴졌어요. 특히 여러 클래스들을 조합하여 새로운 클래스를 만들거나 복잡하게 데이터가 오갈 때 이 장점이 부각될 것 같아요.

enum 사용 소감

음... 솔직히 아직까지는 크게 장점이 와닿지는 않았던 것 같아요. 상수들만을 관리하는 클래스를 만들어서 쓸 수도 있었을 것 같고, 심지어 그게 더 간단한 문법이었을 수도 있을 것 같아요. enum을 어떨 때 쓰면 좋을지, 상수들을 관리하는 클래스를 만들었을 때와는 무슨 차이가 있는지 좀 더 알아봐야겠네요. ~~~.getValue() 남발하다 보니 너무 길어지더라고요. 물론 의미를 나타낸다면 충분히 이름은 길어질 수 있지만 이건 좀 다른 느낌인 것 같아요. getValue() 를 뺀다고 의미가 퇴색되는 것도 아닐테고요.

이번 미션에서는 크게 헤맨 점은 없지만, 번거로운 점은 좀 많았던 것 같아요. 자바스크립트에서는 쉽게 했던 배열 합치기, 배열 그대로 대입하기, 맵핑하기가 자바에서는 상당히 불편하게 느껴졌어요. 아마 코드 곳곳에 복잡한 부분이 있을 거에요. 자바스크립트의 문법이 그리워지네요. 😢

아래는 실행시켜 본 화면이에요.

그럼, 잘 부탁드리겠습니다!

[hafnium1923]

안녕하세요 요토오~~!!! 명절이라고 다음주까지 리뷰를 해도 된다고 하셨지만! 그 김에 이번엔 오래 핑퐁하는 것도 좋아보여서 리뷰를 남깁니다. 아 진짜 토끼는 코딩 천재인가요? 왤케 잘하세요 자바 처음 한다며요;;ㅠ 컬렉션 개념이 확실하게 잡혀있는 것 같아서 감탄했답니다.
저는 아직 제가 쓰고 있는게 컬렉션이 맞나 의문이 들거든요. 그냥 한번 감싼걸 또 감싼다 -> 일급컬렉션
이렇게 이해하고 있는데 맞나요?ㅋㅋㅋㅠ 그래서 저는 로또 숫자도 클래스 만들고 그거로 로또티켓 만드는 형식으로 일급컬렉션 구현하려고 했거든요..
이번 리뷰는 워낙 너무 잘해주셨기도 하고 코드에 의문이 드는 부분이 많이 없어서 enum쪽을 위주로 리뷰해보았습니다.
이외에도 토론하고 싶은 주제가 있다면 언제든 말해주세요!
풀스텍 요토가 앞으로 단 한걸음 남았네요><ㅋㅋ

[hafnium1923]

클래스에서 lottoList 필드를 final로 선언하지 않은 이유가 있나요?
다른 클래스들에서는 보통 final로 선언되어있는 것 같더라구요

[wzrabbit]

미처 못 보고 놓친 부분이에요. final 키워드 추가할게요 🚀

[hafnium1923]

지금은 단순히 상수 값을 저장하는 용도로만 사용되고 있는데, 이런 식으로 enum을 사용한다면 연관된 여러개의 데이터를 함께 처리할 수 있을 것으로 보여요

Suggested change

	FIRST_PRIZE_MONEY(2_000_000_000),
	FIRST(6, 2_000_000_000, "6개 일치"),
	SECOND(5, 30_000_000, "5개 일치, 보너스볼 일치"),

보너스볼 당첨 정보도 함께 다룬다면 로직들이 더 간단해지지 않을까요?

Suggested change

	FIRST_PRIZE_MONEY(2_000_000_000),
	FIRST(6, 2_000_000_000, false,"6개 일치"),
	SECOND(5, 30_000_000, true, "5개 일치, 보너스볼 일치"),

[hafnium1923]

컨트롤러가 너무 많은 상태를 가지고 있는 것 같은데 꼭 필수적인 상태 주입을 제외하고 줄여보는 건 어떨까요?

[wzrabbit]

컨트롤러를 너무 잘게 나누다 보니까 메서드와 메서드 간에 공유해야 하는 값이 있어 사실상 전역과 비슷한 상태로 두게 되어 버렸군요. 상태가 너무 많다는 건 동의합니다 -- 사실상 한 번만 공유하고 끝나는 1회성 필드들은 이렇게 둘 필요는 없었던 것 같아요.

다음에 작업할 때에는 큰 단위의 메소드와 작은 단위의 메소드를 만들고, 큰 단위의 메소드에 지역 변수처럼 선언하는 식으로 변경해볼까 합니다, 혹시 또다른 묘안이 떠오른다면 알려주세요

[hafnium1923]

좋은 것 같아요. 확실히 지금 다 갈아엎기엔 좀 대공사긴 하네요ㅋㅋㅋㅋㅋ

[hafnium1923]

얘도 final 키워드 선언해도 될 것 같아요

[hafnium1923]

구입금액만을 가지고 있는 클래스가 필요한 이유가 무엇일까요?
입력 받은 숫자의 불변성을 보장하고 싶은 것이 의도라고 생각되는데 그렇다면 컨트롤러에 선언되어있는 lottoCount 필드�가 없어야 할 것 같아요. 어떻게 생각하시나요?

[hafnium1923]

일반적인 Error보다는 IllegalArgumentException 와 같은 에러를 사용하는게 좋을 것 같아요

[wzrabbit]

우왁, 이건 제 실수네요. 포괄적으로 적더라도 Exception 일텐데요. 습관이 참 무서워요 🙄

[hafnium1923]

수정하지 못하게 불변 리스트로 반환하는 건 어떤가용?

Suggested change

	return lotto;
	return Collections.unmodifiableList(lotto)

[wzrabbit]

오, Object.freeze 와 비슷한 녀석이 Java에도 있었다니! 감사합니다.

[hafnium1923]

제 생각엔 단순 상수를 enum으로 관리하는 건 약간 비효율적인게 아닐까? 라는 생각이 들어요. 그냥 정적 변수로 선언해도 괜찮을 것 같아요!

[wzrabbit]

궁금한 점이 있어요. 사실 enum 을 쓰면서 들었던 의문이기도 한데, 똑같이 상수를 관리하더라도 LottoConstants 에 대해서는 정적 변수를 더 추천해주시고, (리뷰를 반영한다면) 좀 더 여러 종류의 값을 지닐 LottoPrizeConstants에 대해서는 정적 변수를 더 추천하시는 코멘트를 하지 않으셨는데요

루루는 구조가 좀 복잡한 형태(특히 언급해주신 "연관된 데이터들을 한꺼번에 처리")의 상수 데이터라면 enum을 가져가고, 아니면 정적 변수를 가져가는 편인가요? 어떨 때 enum 도입을 고려하는지 궁금해요! 처음 써 본 입장이라 생소하기도 하고 말이죠

[hafnium1923]

루루는 구조가 좀 복잡한 형태(특히 언급해주신 "연관된 데이터들을 한꺼번에 처리")의 상수 데이터라면 enum을 가져가고,

맞아요! 저는 딱 이생각으로 분리해요. 프론트 개발할 때 관련된 애들끼리 객체배열 만들어서 상수로 관리하잖아요 그런 느낌으로 사용합니다. 단일 값이면 그냥 final키워드로 선언하는게 타입적으로도, 사용할 때도 편하지만 이게 연관된 객체같은 느낌이면 하나하나 선언해서 쓰기 귀찮고 복잡하잖아요. 그리고 속성들끼리 연관적이라는걸 다른 로직으로 검증도 해줘야하구요 (예를들어 매치카운트가 6개면 1등이다 이런식으로의 연결)
그런데 연관된 친구들을 이넘으로 관리한다면 변하지 않는 상수라는 것도 보장해주고, 타입도 안정적이고, 속성끼리의 연결도 필요없으니 더 간단하다고 생각하거든요.

[hafnium1923]

컨트롤러에 DI 와 관련된 부분이 없으니 뭔가 어색해보이네요ㅋㅋ 따로 주입하지 않은 이유가 있나요?

[wzrabbit]

저 이 코멘트는 이해가 잘 안 되었는데, 혹시 가능하시다면 조금 더 자세히 설명해 주실 수 있나요? DI(Dependency Injection)이랑 컨트롤러의 연관성을 잘 떠올리지 못했던 것 같아요 😢

[hafnium1923]

아..! 별건아니고 생성자 넣어보자는 거였어요 제가 스프링 공부하면서는 컨트롤러에 약간 관성적으로 생성자넣어서 객체 주입했거든요ㅋㅋ

[hafnium1923]

이넘을 수정한다면 아래 부분도 훨씬 간단하게 출력 할 수있을 것 같아보여요

[wzrabbit]

@hafnium1923 👋🏻

잘 지내고 계신 것... 맞죠? 늦게 리뷰 반영해서 죄송해요.

모든 걸 반영하지는 못했지만 루루가 미션을 진행할 때 enum에 여러 값을 넣고 관리하는 게 신기해서 enum 코멘트 위주로 리뷰 반영을 진행해 보았어요. EnumMap 이라는 신기한 녀석도 있더군요. Enum의 값을 키로 두고, 순서가 보장되는 자료구조라... 해쉬맵이랑 트리맵밖에 모르는 뜨뜻미지근한 저한테는 신기하게 느껴지는 자료구조였어요. Java에는 별 자료구조가 상당히 많은 것 같아요.

대부분의 코멘트에는 답변을 드렸으니 필요 시 더 이야기해 볼 수 있을 것 같습니다. 항상 코드 비교해보며 인사이트 얻고 있어요, 고마워요!

[wzrabbit]

오, Object.freeze 와 비슷한 녀석이 Java에도 있었다니! 감사합니다.

[wzrabbit]

우왁, 이건 제 실수네요. 포괄적으로 적더라도 Exception 일텐데요. 습관이 참 무서워요 🙄

[wzrabbit]

궁금한 점이 있어요. 사실 enum 을 쓰면서 들었던 의문이기도 한데, 똑같이 상수를 관리하더라도 LottoConstants 에 대해서는 정적 변수를 더 추천해주시고, (리뷰를 반영한다면) 좀 더 여러 종류의 값을 지닐 LottoPrizeConstants에 대해서는 정적 변수를 더 추천하시는 코멘트를 하지 않으셨는데요

루루는 구조가 좀 복잡한 형태(특히 언급해주신 "연관된 데이터들을 한꺼번에 처리")의 상수 데이터라면 enum을 가져가고, 아니면 정적 변수를 가져가는 편인가요? 어떨 때 enum 도입을 고려하는지 궁금해요! 처음 써 본 입장이라 생소하기도 하고 말이죠

[wzrabbit]

미처 못 보고 놓친 부분이에요. final 키워드 추가할게요 🚀

[wzrabbit]

저 이 코멘트는 이해가 잘 안 되었는데, 혹시 가능하시다면 조금 더 자세히 설명해 주실 수 있나요? DI(Dependency Injection)이랑 컨트롤러의 연관성을 잘 떠올리지 못했던 것 같아요 😢

[wzrabbit]

컨트롤러를 너무 잘게 나누다 보니까 메서드와 메서드 간에 공유해야 하는 값이 있어 사실상 전역과 비슷한 상태로 두게 되어 버렸군요. 상태가 너무 많다는 건 동의합니다 -- 사실상 한 번만 공유하고 끝나는 1회성 필드들은 이렇게 둘 필요는 없었던 것 같아요.

다음에 작업할 때에는 큰 단위의 메소드와 작은 단위의 메소드를 만들고, 큰 단위의 메소드에 지역 변수처럼 선언하는 식으로 변경해볼까 합니다, 혹시 또다른 묘안이 떠오른다면 알려주세요

[wzrabbit]

우선 정말 좋은 코멘트 감사합니다. 코멘트를 계기로 고민해 볼 기회가 생긴 것 같습니다.

한 줄로 요약하면, 여기에서는 컬렉션을 만들어 쓰기에는 다소 과했다는 생각이 들었었기 때문입니다.

입력값에 대한 검증을 컨트롤러에서 진행하고 있는 이유

해당 입력값과 관련된 클래스에서 검증하지 않은 이유

우선 이 입력값 검증의 경우 둘 이상의 요소가 연관되어 있는 상태입니다. 1) 수동 로또의 개수(입력받는 값), 2) 전체 로또의 개수(검증 시 사용하게 되는 값) 입니다. 값을 컬렉션으로 관리하고 싶을 경우 컬렉션을 만들고 그 컬렉션 내에 검증 로직을 두면 될 것 같습니다.

다만 저는 여기에서는 컬렉션을 또 만들어 (수동 로또의 개수 / 전체 로또의 개수)를 묶는 컬렉션을 만들거나, (수동 로또의 개수) 단일 값을 관리하는 컬렉션을 만들고 싶지 않았습니다.

(수동 로또의 개수 / 전체 로또의 개수)를 묶는 컬렉션을 만들고 싶지 않은 이유

• 전체 로또 개수의 경우 이미 컬렉션으로 두고 있는 LottoList 의 size() 를 이용하면 값을 받아올 수 있습니다. LottoList 는 이미 올바른 값만을 가지는 것이 보장되므로 로또의 개수만을 관리하는 컬렉션을 따로 만들 필요성을 느끼지 못했습니다. 추후 수동 로또/전체 로또의 개수가 여러 번 필요하다면 이미 이 기능을 수행하고 있는 컬렉션으로부터 받아오지, 따로 컬렉션을 또 만들어 관리하지는 않을 것 같습니다.
• 첫 검증을 제외하고는 두 로또의 개수를 묶어놓고 관리할 필요성을 크게 못 느꼈습니다. 각 로또의 개수는 각각의 LottoList 컬렉션의 생성에 활용되며, 컬렉션의 생성 이후에는 위에서도 이미 이야기했지만 size()를 이용해 관리가 가능합니다.

(수동 로또의 개수)만을 묶는 컬렉션을 만들고 싶지 않은 이유

• 수동 로또의 개수만을 저장하는 컬렉션을 만들어도, 결국 검증에는 전체 로또의 개수가 필요합니다.
• 그러나 검증 조건이 단순합니다. 수동 로또의 개수 > 전체 로또의 개수 만 아니면 되거든요. 그런데 이 조건 하나의 검사를 위해 수동 로또를 주입하고 복잡하게 관리하고 싶지는 않았습니다.
• 검증하고 땡! 이더라도, 이 수동 로또의 개수 값 자체가 빈번히 쓰인다면 컬렉션으로 둘 가치는 충분하다고 생각합니다. 그런데 그 역할은 이미 LottoList 컬렉션의 size() 메소드가 해 주고 있습니다. 수동 로또 또한 LottoList로 관리하거든요.

그래서 컨트롤러에 로직을 두게 되었습니다. 두 값을 연관시켜 강하게 결합시킨 채로 관리하고 싶지 않고, 이미 존재하는 다른 컬렉션이 상당 부분 이 일을 대신해주고 있기 때문입니다.

그 외에도 컨트롤러에 로직을 둘 이유를 생각해 보았는데 두 모델의 도메인 성격이 꽤나 떨어진 경우에도 컨트롤러에 검증 로직을 둘 것 같습니다.

어떤 기준으로 검증에 대한 로직을 각 모델과 컨트롤러에 분산시켜 위치한 것인지

• 검증 로직을 컬렉션에 둘 경우 내부에 두는 값을 안전하게 보호할 수 있습니다. 그런데, 컬렉션이 한 번만 사용되어 보호의 필요성이 크게 느껴지지 않거나, 컨트롤러로 1회성 검증만 해도 충분한 경우에는 컬렉션을 사용하고 도메인에 검증 로직을 둘 지, 그렇지 않을지 고민할 것 같습니다.

• 검증 시에 여러 개의 값이 검증 성공 여부에 관여하는 경우, 이 여러 개의 값들을 컬렉션으로 묶어 강력하게 결합시키고 싶지 않을 경우에는, 컬렉션으로 만들지 않고 컨트롤러에 검증 로직을 둘 것 같습니다.

이상 제가 고민하고 생각해 본 것들의 결론입니다. 제 말이 무조건 맞다고 생각하지 않으며 코멘트를 적을 당시 조금이나마 알고 있는 지식과 생각을 바탕으로 적은 것이므로 의견이 갈릴만한 부분이 많은 것도 맞습니다.
혹시 다른 생각을 가지고 계시다면 공유해주셔도 좋습니다!

[hafnium1923]

코드 반영 진짜 깔끔하네요우~ 제안하고 싶은건 더 보이지 않아서 approve눌렀지만 더 토론이 필요하다면 언제든지 환영입니당!

[hafnium1923]

좋은 것 같아요. 확실히 지금 다 갈아엎기엔 좀 대공사긴 하네요ㅋㅋㅋㅋㅋ

[hafnium1923]

아..! 별건아니고 생성자 넣어보자는 거였어요 제가 스프링 공부하면서는 컨트롤러에 약간 관성적으로 생성자넣어서 객체 주입했거든요ㅋㅋ

[hafnium1923]

오,, 되게 좋은 인사이트네요,,검증 로직을 도메인 객체 내부에 두어 컬렉션이 항상 신뢰할 수 있는 상태임을 보장할 수도 있지만, 이번 경우처럼 두 값이 서로 연관되어 있고 이미 다른 컬렉션으로 관리되고 있다면 컨트롤러에서 검증하는 것도 충분히 합리적이라고 생각돼요. 자세하게 정리해주셔서 감사합니룽~

[hafnium1923]

루루는 구조가 좀 복잡한 형태(특히 언급해주신 "연관된 데이터들을 한꺼번에 처리")의 상수 데이터라면 enum을 가져가고,

맞아요! 저는 딱 이생각으로 분리해요. 프론트 개발할 때 관련된 애들끼리 객체배열 만들어서 상수로 관리하잖아요 그런 느낌으로 사용합니다. 단일 값이면 그냥 final키워드로 선언하는게 타입적으로도, 사용할 때도 편하지만 이게 연관된 객체같은 느낌이면 하나하나 선언해서 쓰기 귀찮고 복잡하잖아요. 그리고 속성들끼리 연관적이라는걸 다른 로직으로 검증도 해줘야하구요 (예를들어 매치카운트가 6개면 1등이다 이런식으로의 연결)
그런데 연관된 친구들을 이넘으로 관리한다면 변하지 않는 상수라는 것도 보장해주고, 타입도 안정적이고, 속성끼리의 연결도 필요없으니 더 간단하다고 생각하거든요.