[오치현] 1차 과제 제출 #9

ch1hyun · 2024-10-01T03:26:41Z

No description provided.

- Factory Pattern 적용

- 함수명 변경 : findNameToEnumIn -> from - 인자 변경 : String itemName -> Item item

hades3

저도 상수화하는 습관을 들여야겠다고 생각했습니다. 잘 읽었습니다.

hades3 · 2024-10-06T11:03:03Z

Java/src/main/java/com/gildedrose/GildedRose.java

+                && !items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) {
+                if (items[i].quality > MINIMUM_QUALITY.getValue()) {


상수화하는 습관은 정말 좋은 것 같습니다.
다만, 개인적으로 static 변수로 사용하는 것이 더 깔끔할 것 같습니다. 참고만 해주세요!

hades3 · 2024-10-06T11:06:58Z

commit 메세지를 잘 써주셔서, 어떤 변경이 이루어졌는지 파악하기 편했습니다.

supsup-hae · 2024-10-07T13:58:22Z

커밋 컨벤션 관리하신 게 굉장히 인상적이에요! 주로 어떤 기준으로 Commit 단위를 지정하시나요?

chaen-ing · 2024-10-09T09:15:19Z

상수를 패키지로 관리하는 부분이 인상깊었습니다. 이외에도 팩토리를 통해서 코드를 잘 분리하신 것 같아요! 커밋컨벤션도 많이 배워갑니다

chanmin-00 · 2024-10-10T05:37:54Z

Java/src/main/java/com/gildedrose/constant/ItemNameConstant.java

+    public static ItemNameConstant from(Item item) {
+        return Arrays.stream(ItemNameConstant.values())
+            .filter(modifier -> item.name.contains(modifier.name))
+            .findFirst()
+            .get();
+    }


저는 stream을 사용하는 부분에 대하여 생각하지 못했는데, stream을 사용한 것을 보니 코드에 대한 가독성이 좋아서 읽기가 편했습니다

이 부분을 보고 감탄했습니다. stream으로 쓰니 가독성이 정말 좋네요.

chanmin-00 · 2024-10-10T05:40:30Z

Java/src/main/java/com/gildedrose/constant/ItemPropertyConstant.java

@@ -0,0 +1,19 @@
+package com.gildedrose.constant;
+
+public enum ItemPropertyConstant {


코드에서 자주 사용되는 값들을 상수화한 것을 보니 추후 코드 변경이 이루어질 경우 유지 보수하기가 편할거 같다는 생각이 듭니다.

chanmin-00 · 2024-10-10T05:47:13Z

Java/src/main/java/com/gildedrose/update/UpdateFactory.java

+import java.util.concurrent.ConcurrentHashMap;
+
+public class UpdateFactory {
+    private final Map<ItemNameConstant, Update> updateMap = new ConcurrentHashMap<>();


Map 방식은 생각해보지 못했는데, 이러한 방식을 통해 각각의 Item 값에 따라 해당하는 Update 함수를 제공할 수 있도록 작성하셔서 인상 깊게 보았습니다. 이렇게 작성을 하게 되면 추후에 코드를 확장하기도 용이하다는 생각이 듭니다

chanmin-00 · 2024-10-10T05:51:37Z

Java/src/main/java/com/gildedrose/update/BackstagePassesUpdate.java

+    @Override
+    public void update(Item item) {
+        if (item.sellIn <= BACKSTAGE_DAY_OF_5.getValue()) {
+            item.quality = Math.min(item.quality + 3, MAXIMUM_QUALITY.getValue());


숫자의 크기를 비교하는 Math 함수의 메소드 코드가 반복되고 있는데 이런것들을 함수화해서 코드를 간소화하는 것도 좋은 방법일 거 같아요

구현하면서 고민했던 부분인데 짚어주셔서 감사해요!
리팩토링에서 찬민님이 말씀해주신대로 구현해봐야겠어요 :)

chanmin-00

Map 방식을 활용하여 각 아이템마다 update함수를 관리하는 방식과 상수화 하는 방식들이 인상 깊었습니다. 수고하셨습니다!

yooooonshine · 2024-10-10T10:08:19Z

Java/src/main/java/com/gildedrose/update/UpdateFactory.java

+import java.util.concurrent.ConcurrentHashMap;
+
+public class UpdateFactory {
+    private final Map<ItemNameConstant, Update> updateMap = new ConcurrentHashMap<>();


혹시 이 부분 왜 ConcurrentHashMap을 사용하셨는 지 궁금합니다!
처음 보는 자료구조라 알아봤는 데, 주로 스레드 안전, 멀티스레드에서 사용되는 자료구조라고 하더라구요!
다만, 제가 봤을 때는 updateMap은 서버가 실행되면 변경 작업이 없어 락이 걸릴 일이 없을 거 같아서, 의도가 궁금합니다!

업데이트하면 아이템 값이 수정되어 동시성 문제가 발생할거라 막연하게 생각했는데, 말씀 듣고보니 업데이트 자체의 변경은 없는 것 같아요! 쓸 이유가 없었네요.
생각하지 못했던 부분 짚어주셔서 감사합니다 :)

goalSetter09

코드 보고 정말 많이 배워갑니다~ 수고하셨습니다!

goalSetter09 · 2024-10-10T11:03:46Z

Java/src/main/java/com/gildedrose/constant/ItemNameConstant.java

+    public static ItemNameConstant from(Item item) {
+        return Arrays.stream(ItemNameConstant.values())
+            .filter(modifier -> item.name.contains(modifier.name))
+            .findFirst()
+            .get();
+    }


이 부분을 보고 감탄했습니다. stream으로 쓰니 가독성이 정말 좋네요.

ch1hyun · 2024-10-10T14:14:12Z

커밋 컨벤션 관리하신 게 굉장히 인상적이에요! 주로 어떤 기준으로 Commit 단위를 지정하시나요?

주로 기능 단위로 작성을 하는데요, 기능이라고 하면 모호한 것 같아 곰곰히 생각해봤습니다.
깃을 사용하는 이유가 버전 관리이기도 하지만, 이전 상태로 돌아가기 위함임을 생각해봤을 때 이전 상태로 돌아간다는 것은 그 코드가 정상적으로 동작할 수 있어야 한다는 것과 같다고 생각해요.
이런 관점에서 하나의 기능을 구현하고 실행했을 때 올바르게 동작하면 Commit을 날리고 있습니다 ㅎㅎ

0-x-14

코드 잘 봤습니다!

0-x-14 · 2024-10-11T08:03:10Z

Java/src/main/java/com/gildedrose/update/AgedBrieUpdate.java

+        item.quality = Math.min(item.quality + 1, MAXIMUM_QUALITY.getValue());
+
+        if (item.sellIn < SELLIN_CRITERIA.getValue()) {
+            item.quality = Math.min(item.quality + 1, MAXIMUM_QUALITY.getValue());


Math 함수로 최댓값을 제한해서 구현하니 코드가 훨씬 깔끔하네요! 생각해보지 못한 방법인데 배워갑니다

0-x-14 · 2024-10-11T08:04:32Z

Java/src/main/java/com/gildedrose/update/BackstagePassesUpdate.java

+public class BackstagePassesUpdate implements Update {
+    @Override
+    public void update(Item item) {
+        if (item.sellIn <= BACKSTAGE_DAY_OF_5.getValue()) {


남은 날짜를 상수화해서 작성하니 한눈에 알아보기 좋네요!

ch1hyun added 9 commits October 1, 2024 01:02

build: Naver Code Style 추가 및 Gradle 버전 업그레이드(8.8)

283601f

test: Conjured 아이템 테스트 추가

26f567c

style: 아이템 값 상수화

de48b1c

style: public 접근 지정자 누락

7c980b7

style: 아이템 이름 상수화

69e9f4b

refactor: 아이템 수식어 별 업데이트 로직 변경

9050ca0

- Factory Pattern 적용

style: 주석 삭제

b5345cb

style: 불필요한 import 삭제

f6d357f

refactor: ItemNameConstant 인스턴스 반환 함수명 및 인자 변경

0f2c9ae

- 함수명 변경 : findNameToEnumIn -> from - 인자 변경 : String itemName -> Item item

hades3 reviewed Oct 6, 2024

View reviewed changes

chanmin-00 reviewed Oct 10, 2024

View reviewed changes

yooooonshine reviewed Oct 10, 2024

View reviewed changes

goalSetter09 reviewed Oct 10, 2024

View reviewed changes

ch1hyun closed this Oct 10, 2024

ch1hyun reopened this Oct 10, 2024

0-x-14 reviewed Oct 11, 2024

View reviewed changes

ch1hyun added 6 commits October 19, 2024 23:54

refactor(Update): 업데이트 인터페이스 추상 클래스로 변경

94d818f

refactor(Update): 날짜 감소 공통 로직 추출

94aed02

refactor(Update): 가치 증감 연산 공통 로직 추출

e20dd57

refactor(BackstagePassesUpdate): 메서드 분리

ef18fd9

refactor(BackstagePassesUpdate): 날짜에 따른 가치 값 변경의 공통 로직 추출

3b3c756

refactor(AllUpdate): 증감폭 수치 상수화

80d63f7

ch1hyun added 7 commits October 20, 2024 00:23

refactor(Update): ItemPropertyConstant static import로 변경

f0484bd

refactor(Constants): 상수 파일 이름 변경

f64ea55

refactor(ItemName): Optional에서 NULL을 검사하지 않고 반환하는 것을 수정

33a6240

fix(Update): 증감에 따른 가치 값에 지정한 한계가 적용되지 않는 것을 수정

31abc63

refactor(UpdateFactory): 동시성 문제가 필요없어 HashMap 클래스로 변경

219e7e3

refactor(UpdateFactory): update를 updateHandler로 이름 변경

fa36b92

style(AllFile): 사용하지 않는 import 제거 및 컨벤션에 따른 수정

663c4e1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[오치현] 1차 과제 제출 #9

[오치현] 1차 과제 제출 #9

ch1hyun commented Oct 1, 2024

hades3 left a comment

hades3 Oct 6, 2024

hades3 commented Oct 6, 2024

supsup-hae commented Oct 7, 2024

chaen-ing commented Oct 9, 2024

chanmin-00 Oct 10, 2024

goalSetter09 Oct 10, 2024

chanmin-00 Oct 10, 2024

chanmin-00 Oct 10, 2024

chanmin-00 Oct 10, 2024

ch1hyun Oct 10, 2024

chanmin-00 left a comment

yooooonshine Oct 10, 2024

ch1hyun Oct 10, 2024

goalSetter09 left a comment

goalSetter09 Oct 10, 2024

ch1hyun commented Oct 10, 2024

0-x-14 left a comment

0-x-14 Oct 11, 2024

0-x-14 Oct 11, 2024

		&& !items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) {
		if (items[i].quality > MINIMUM_QUALITY.getValue()) {

		@@ -0,0 +1,19 @@
		package com.gildedrose.constant;

		public enum ItemPropertyConstant {

[오치현] 1차 과제 제출 #9

Are you sure you want to change the base?

[오치현] 1차 과제 제출 #9

Conversation

ch1hyun commented Oct 1, 2024

hades3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hades3 commented Oct 6, 2024

supsup-hae commented Oct 7, 2024

chaen-ing commented Oct 9, 2024

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

chanmin-00 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goalSetter09 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ch1hyun commented Oct 10, 2024

0-x-14 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment