thread upload #43

nueag · 2023-11-06T01:54:02Z

No description provided.

NANNDA3463 · 2023-11-06T04:33:35Z

Thread/src/main/java/com/nhnacademy/gaeun/assignment01/product/CannedFoods.java

+
+import org.example.thread.assignment01.Product;
+
+public class CannedFoods extends Product {


name과 amount만 달라지는 클래스라면 팩토리 같은 생성자를 활용했으면 더 좋았을 것 같습니다

NANNDA3463 · 2023-11-06T04:35:31Z

Thread/src/main/java/com/nhnacademy/gaeun/assignment01/Main.java

@@ -0,0 +1,21 @@
+package org.example.thread.assignment01;


다음엔 패키지 명도 리팩토링 해주세요

NANNDA3463 · 2023-11-06T04:40:53Z

Thread/src/main/java/com/nhnacademy/gaeun/assignment01/Product.java

+        return name;
+    }
+
+    public synchronized boolean isAbleBuy(int amount) {


Product가 판매하거나 구입할 수 있는 지를 판단하거나 판매하는 것이 맞을 지 모르겠습니다.

FoodStand에서 결국 해당 Product의 sell을 호출하는 식이라면, 이곳이 아니라 FoodStand에 있으면 어떨까 싶습니다

저도 동의합니다! 저는 buy라는 게 사는 행동이기 때문에 Store가 판별하는게 괜찮을 거 같다라는 생각인데 어떻게 생각하시나용?

저도 다시 생각해봤는데 수정하려고 보니까 foodstand는 그냥 여러 매장을 담고 있는 가판대 역할을 하고 foodstand 안에 필드로 product 리스트가 각각 건어물 매장, 수산물 매장, 등등의 역할을 하고 있습니다. 이름 때문에 헷갈리는 것 같아서 이름만 수정했습니다. 혹시 이렇게 본다면 의미상 괜찮을까요?? 기존의 product가 각각의 매장을 뜻하고 건어물 매장 등이 이를 상속한다면요 ! Product -> Store로, Store -> Mart로 수정했습니다!

NANNDA3463 · 2023-11-06T04:44:15Z

Thread/src/main/java/com/nhnacademy/gaeun/assignment01/Store.java

+    }
+
+    public synchronized void exit(Consumer consumer) {
+        if (people.contains(consumer)) {


List의 contains는 비교하려는 해당 객체의 equals를 통해 판단한다고 합니다.

Consumer 클래스에 equals를 넣는다면 더욱 활용이 완벽해질 것 같습니다

이런 유형으로 Product를 비교하는 것도 있었으니 한번 찾아보면 좋을 듯 싶습니다

NANNDA3463 · 2023-11-06T04:46:23Z

Thread/src/main/java/com/nhnacademy/gaeun/assignment01/Producer.java

+public class Producer implements Runnable {
+    private Store store;
+    private List<Product> itemList;
+    private Random random;


생성자에서 초기화 해줘도 좋을 것 같습니다

수정했습니다!

NANNDA3463 · 2023-11-06T04:48:23Z

Thread/src/main/java/com/nhnacademy/gaeun/assignment01/Consumer.java

+
+    public Consumer(String name, Store store) {
+        this.store = store;
+        this.name = name + ++count;


비직관적인 코드인것 같습니다
둘을 분리해주세요

분리했습니다.

NANNDA3463 · 2023-11-06T04:54:49Z

Thread/src/main/java/com/nhnacademy/gaeun/assignment01/FoodStand.java

+        if(productList.contains(product)) {
+            product.add(amount);
+        } else {
+            productList.add(product);


물건 리스트에 추가하고, 그 물건의 amount도 add해주는 식이면 더 의도에 맞을 수도 있겠습니다

product클래스 내부의 add메서드에서 구현하고 있습니다

NANNDA3463 · 2023-11-06T04:57:46Z

Thread/src/main/java/com/nhnacademy/gaeun/example/Consumer.java

+
+    public Consumer(String name, Store store) {
+        this.store = store;
+        this.name = name + count++;


count++ 부분을 분리하면 더 좋을 것 같습니다

맞아요 코드를 보면 Consumer 인스턴스를 몇개 생성하는지 count를 해주고 싶으신 것 같으신데
count를 증가 시키는 메서드를 분리하면 좀 더 직관적이지 않을까요??

수정했습니다!

NANNDA3463 · 2023-11-06T04:58:25Z

Thread/src/main/java/com/nhnacademy/gaeun/example/Product.java

@@ -0,0 +1,4 @@
+package org.example.thread.example;
+
+public class Product {


TA님 왈 필요없는 클래스는 지우는 게 좋다고 합니다

store클래스에서 사용되고 있습니다!

minsu11 · 2023-11-06T13:14:22Z

Thread/src/main/java/com/nhnacademy/gaeun/assignment01/Consumer.java

+    private Store store;
+    private String name;
+    private Random random;
+    public static int count = 0;


static int로 하신 이유가 있을까여?? 그리고 접근 제어자를 public으로 하신 이유가 있을까여?

Consumer 인스턴스가 몇개 생성하는지 count 하고 싶으셔서 하는게 아닐까요?? 접근제어자는 이 변수가 이 클래스에서만 쓰인다면 private로 해주시는게 좋지 않을까요?

consumer가 새로 생성되더라도 count수는 유지하기 위해서 static 사용했습니다.

접근제어자 private로 수정했습니다.

minsu11 · 2023-11-06T15:13:01Z

Thread/src/main/java/com/nhnacademy/gaeun/assignment01/Main.java

+import java.util.concurrent.TimeUnit;
+
+public class Main {
+    public static void main(String[] args) throws InterruptedException {


DeadLock이 걸리는 경우가 생기는 것같습니다! 손님이 계속 대기 중으로만 출력값이 나오는 경우가 생깁니다!

오! 수정했습니다!

minsu11 · 2023-11-06T15:46:00Z

Thread/src/main/java/com/nhnacademy/gaeun/assignment01/Store.java

+
+    private FoodStand foodStand;
+
+    public static final int PEOPLE_NUM_LIMIT = 5;


해당 필드의 접근제어자를 public으로 하신 이유가 있을까요?? 그리고 public static final을 해주신 이유가 있을까용? 제 생각에는 private final int가 괜찮을거 같은데 어떻게 생각하시나용?

또한 int 값을 사용해주셔도 괜찮지만, 그러면 해당 int의 값에 대한 동시성 제어도 신경을 써줘야하는 불편함이 생길 수도 있을 것 같습니다!
동시성 제어를 해주는 AtomicInteger를 사용하는 건 어떨까요??

해당 상수는 인원수 제한을 목적으로 하신 것 같은데 이 상수가 이 클래스에서만 쓰인다면 private로 선언 해주시면 좋을 것 같아요

ms님!!
두번째 comment에서 동시성 제어를 해줘야 한다고 하셨는데 이 상수는 그저 인원수 제한을 목적으로 만드신 것 같아서 동시성 제어는 안해줘도 될 것 같습니다!!
그리고 private static final로 하는게 맞지 않을까요?
이유는
만약 private final int로 지정을하게 되면
Store 를 여러 인스턴스를 만들게 되면 각 인스턴스마다 저 상수를 각자 가지고 있습니다 이말은 인스턴스마다 저 상수의 메모리를 차지하는 것인데 비효율적이지 않을까요?
static 키워드를 하면 모든 인스턴스에서 이 상수 하나만을 공유하게 되어서 메모리 사용량이 줄어 들기 때문에 설정하는게 좋은 것 같습니다!!!

private으로 수정하겠습니다!
두분 의견 감사합니다!! 상수값이기 때문에 동시성 제어는 따로 하지 않았습니다.

hyeonjaez · 2023-11-06T16:24:44Z

Thread/src/main/java/com/nhnacademy/gaeun/assignment01/Consumer.java

+
+    public Consumer(String name, Store store) {
+        this.store = store;
+        this.name = name + ++count;


count를 증가시키는 메서드 만들어줘서 분리 시키면 좋을 것 같습니다 어떻게 생각하시나요?

좋은 의견인 것 같습니다.

hyeonjaez · 2023-11-06T16:27:26Z

Thread/src/main/java/com/nhnacademy/gaeun/assignment01/Store.java

+
+    private FoodStand foodStand;
+
+    public static final int PEOPLE_NUM_LIMIT = 5;


해당 상수는 인원수 제한을 목적으로 하신 것 같은데 이 상수가 이 클래스에서만 쓰인다면 private로 선언 해주시면 좋을 것 같아요

hyeonjaez · 2023-11-06T16:36:50Z

Thread/src/main/java/com/nhnacademy/gaeun/assignment01/Store.java

+
+    private FoodStand foodStand;
+
+    public static final int PEOPLE_NUM_LIMIT = 5;


ms님!!
두번째 comment에서 동시성 제어를 해줘야 한다고 하셨는데 이 상수는 그저 인원수 제한을 목적으로 만드신 것 같아서 동시성 제어는 안해줘도 될 것 같습니다!!
그리고 private static final로 하는게 맞지 않을까요?
이유는
만약 private final int로 지정을하게 되면
Store 를 여러 인스턴스를 만들게 되면 각 인스턴스마다 저 상수를 각자 가지고 있습니다 이말은 인스턴스마다 저 상수의 메모리를 차지하는 것인데 비효율적이지 않을까요?
static 키워드를 하면 모든 인스턴스에서 이 상수 하나만을 공유하게 되어서 메모리 사용량이 줄어 들기 때문에 설정하는게 좋은 것 같습니다!!!

hyeonjaez · 2023-11-06T16:40:26Z

Thread/src/main/java/com/nhnacademy/gaeun/example/Consumer.java

+
+    public Consumer(String name, Store store) {
+        this.store = store;
+        this.name = name + count++;


맞아요 코드를 보면 Consumer 인스턴스를 몇개 생성하는지 count를 해주고 싶으신 것 같으신데
count를 증가 시키는 메서드를 분리하면 좀 더 직관적이지 않을까요??

hyeonjaez · 2023-11-06T16:44:41Z

Thread/src/main/java/com/nhnacademy/gaeun/example/Consumer.java

+import java.util.concurrent.ThreadLocalRandom;
+
+public class Consumer implements Runnable {
+    Store store;


접근제어자를 이렇게 하신 이유가 있을까요?

수정했습니다!

minsu11 · 2023-11-06T23:50:49Z

Thread/src/main/java/com/nhnacademy/gaeun/assignment01/Producer.java

+        }
+    }
+
+    private void randomDelivery() {


randomDelivery가 배달하는 행동이라고 생각을 하는데, 메서드 안에서 store.getFoodStand를 호출해서 add를 하는 방식 대신에 product를 return을 시켜주는 방식은 어떨까요??

그리고 store에서 return 값을 받는 메서드를 만든 다음에 Producer run메서드에 store."메서드 명"(randomDelivery()) 이런 방식으로 해주는건 어떨까여?

오! 참고해서 수정했습니다.

minsu11

parkminsu code review

nueag added 2 commits November 6, 2023 10:52

thread upload

6174691

modify

9aff05a

NANNDA3463 reviewed Nov 6, 2023

View reviewed changes

modify

e707cd8

minsu11 reviewed Nov 6, 2023

View reviewed changes

hyeonjaez reviewed Nov 6, 2023

View reviewed changes

minsu11 reviewed Nov 6, 2023

View reviewed changes

modify

aa180b0


		import org.example.thread.assignment01.Product;

		public class CannedFoods extends Product {

		@@ -0,0 +1,4 @@
		package org.example.thread.example;

		public class Product {


		private FoodStand foodStand;

		public static final int PEOPLE_NUM_LIMIT = 5;

thread upload #43

Are you sure you want to change the base?

thread upload #43

Conversation

nueag commented Nov 6, 2023

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

minsu11 Nov 6, 2023 • 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

minsu11 left a comment

Choose a reason for hiding this comment

minsu11 Nov 6, 2023 •

edited

Loading