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

[7주차]COW-Spring-1 학습(fuirian) #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fuirian
Copy link

@fuirian fuirian commented Aug 29, 2023

image

Copy link
Member

@wonjunYou wonjunYou left a comment

Choose a reason for hiding this comment

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

모든 로직이 view에서 흘러가고 있다는 생각이 듭니다. controller가 전체적인 흐름을 담당하고, 데이터를 가져오는 로직은 도메인과 퍼시스턴스 레이어를 적절히 활용하여 이루어지도록 분리해주세요. view와 로직의 분리가 없으면, 결코 mvc라고 할 수 없습니다.


GameController gameController = new GameController(carNames);

for (int i = 0; i < tryCount; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

stream을 적용해볼 수 있을 것 같아요!

List<String> carNames = Arrays.asList(carNamesInput.split(","));

int tryCount;
try {
Copy link
Member

Choose a reason for hiding this comment

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

시도 횟수를 입력받는 부분도 좀 더 작게 메서드로 분리할 수 있을 것 같습니다!

return;
}

GameController gameController = new GameController(carNames);
Copy link
Member

Choose a reason for hiding this comment

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

뷰에서 컨트롤러를 생성해주는 방식은 옳지 않은 것 같습니다!

List<String> carNames = gameController.getCarNames();
List<Integer> carPositions = gameController.getCarPositions();

for (int i = 0; i < carNames.size(); i++) {
Copy link
Member

Choose a reason for hiding this comment

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

STREAM 적용!


public class Randoms {

public static int pickNumberInRange(int i, int j) {
Copy link
Member

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.

2 participants