[BE][team-10] Todo-list 1주차 PR #1

[CMSSKKK]

처음에 뵙겠습니다 solar🎉

로니&반스입니다!

아직 처음이라 많은 작업을 하지는 못했지만 1차 PR을 보냅니다.
잘부탁드립니다.

구현사항

• DB 테이블 설계 및 쿼리 작성
  • table 설계(프로토타입)
  • schema.sql 작성

DROP TABLE IF EXISTS column;
DROP TABLE IF EXISTS `user`;
DROP TABLE IF EXISTS card;
DROP TABLE IF EXISTS log;

CREATE TABLE column
(
    id       INTEGER AUTO_INCREMENT NOT NULL,
    name     VARCHAR(255) NOT NULL,
    PRIMARY KEY (id)
);

CREATE TABLE `user`
(
    id       VARCHAR(255) NOT NULL,
    name     VARCHAR(255) NOT NULL,
    password VARCHAR(255) NOT NULL,
    PRIMARY KEY (id)
);

CREATE TABLE card
(
     id BIGINT AUTO_INCREMENT NOT NULL,
     user_id VARCHAR(255) NOT NULL,
     column_id INTEGER NOT NULL,
     subject VARCHAR(255) NOT NULL,
     contents VARCHAR(255) NOT NULL,
     create_time TIMESTAMP NOT NULL,
     update_time TIMESTAMP,
     deleted boolean DEFAULT FALSE,
     PRIMARY KEY (id),
     FOREIGN KEY (user_id) REFERENCES `user` (id),
     FOREIGN KEY (column_id) REFERENCES column (id)
);

CREATE TABLE log
(
    id BIGINT AUTO_INCREMENT NOT NULL,
    user_id VARCHAR(255) NOT NULL,
    subject VARCHAR(255) NOT NULL,
    `from` INTEGER,
    `to` INTEGER,
    active_time TIMESTAMP NOT NULL,
    activity VARCHAR(255) NOT NULL,
    PRIMARY KEY (id),
    FOREIGN KEY (user_id) REFERENCES `user` (id),
    FOREIGN KEY (`from`) REFERENCES column (id),
    FOREIGN KEY (`to`) REFERENCES column (id)
);

• Card mockup API 구현

  • Card 객체 생성
  • mockupController 구현 (Json 데이터처리만 시험삼아 구현해봤습니다.)
  • Card Repository 생성
  • Card Service 생성
  • Card Controller 생성
  • Test 코드 작성 (몇가지 단위테스트를 진행했으나 완성하지 못했습니다.)

약속된 시간보다 조금 늦게 PR보내게되어서 죄송합니다 🙇‍♂️

[blossun]

안녕하세요 로니 반스😄 이번에 리뷰를 맡게된 Solar입니다.
첫 PR 고생 많으셨습니다~ 커밋 단위가 잘 나눠져 있어서 리뷰하기 좋았습니다. 👍
테스트 작성하려는 모습도 좋습니다. 😄
한 번 고민해보시면 좋을만한 내용 코멘트로 남겨드렸어요~
수고 많으셨고 남은 미션도 화이팅입니다!!

[blossun]

Rest API 디자인 가이드에 대해서 학습해보시길 추천드립니다~!
링크 하나 첨부해드릴게요 :)
https://meetup.toast.com/posts/92

[ffinn92]

키워드와 링크 공유해주셔서 감사합니다
참고해서 학습해보겠습니다. solar! 🙏

[blossun]

Setter가 필요한가요??

[blossun]

이 변수만 스네이크 케이스이네요?!

[ffinn92]

앗 넵! 수정하겠습니다. 감사합니다.

[blossun]

수정이 필요해보여요~

[ffinn92]

네 수정하여 반영하도록 하겠습니다. 감사합니다.

[blossun]

여기서 setter를 사용하시는군요!
Setter 메소드의 사용은 지양하도록 권고드리는데요.
setter는 왜 사용을 지양해야하는지 고민해보시면 좋을 것 같습니다. 😄

간단히 적어보자면
대표적인 이유로 값을 변경한 의도를 파악하기 힘들다는 점과 객체의 일관성을 유지하기 어렵다는 점이 있습니다.
setter대신 생성자나 정적 팩토리 메소드, 또는 목적과 의도가 분명히 나타낼 수 있는 메소드를 추가해주는 것도 좋은 방법입니다.

[CMSSKKK]

setter를 제거하고 정적펙토리메서드로 바꾼다고 생각했을 때, 이렇게 메서드 구상해봤는데... 괜찮은 메서드인지 잘 모르겠습니다ㅠㅠ

public static Card instanceWithId(Long id, Card card) {
        card.id = id;
        return card;
    }

번거로우시겠지만, 한번 확인해주신다면 해당 내용 반영하도록 하겠습니다!
감사합니다!

[blossun]

지금같은 경우는 특히 엔티티 클래스이기 때문에 변경한 의도를 파악하기 힘들다보다 객체의 일관성을 유지하기 어렵다라는 포인트에서 Setter 사용을 지양하시길 리뷰드렸습니다.
필요하다면 setter를 사용할 수도 있습니다. 하지만 지금의 경우 엔티티를 식별하는 id가 변경될 수 있는데요. 이때 문제가 발생할 여지가 있을까요? 있다면 왜 문제가 될까요?
식별자 가변성에 대해서 고민해보시길 바랍니다.

지금 요구드리는 것은 아니고 추가적으로 불변 객체에 대해서 한 번 공부해보세요 😊

[blossun]

삭제요청이니 deleted = true가 되어야하지 않나요~?

[ffinn92]

네 맞습니다. 수정하겠습니다. 감사합니다.

[blossun]

공백과 들여쓰기를 적절히 사용해서 코드를 읽기 쉽게 하면 좋을 것 같아요~ 예시는 참고만 해주세요!

Suggested change

	id BIGINT AUTO_INCREMENT NOT NULL,
	user_id VARCHAR(255) NOT NULL,
	column_id INTEGER NOT NULL,
	subject VARCHAR(255) NOT NULL,
	contents VARCHAR(255) NOT NULL,
	create_time TIMESTAMP NOT NULL,
	update_time TIMESTAMP,
	deleted boolean DEFAULT FALSE,
	PRIMARY KEY (id),
	FOREIGN KEY (user_id) REFERENCES `user` (id),
	FOREIGN KEY (column_id) REFERENCES column (id)
	id BIGINT AUTO_INCREMENT NOT NULL,
	user_id VARCHAR(255) NOT NULL,
	column_id INTEGER NOT NULL,
	subject VARCHAR(255) NOT NULL,
	contents VARCHAR(255) NOT NULL,
	create_time TIMESTAMP NOT NULL,
	update_time TIMESTAMP,
	deleted boolean DEFAULT FALSE,
	PRIMARY KEY (id),
	FOREIGN KEY (user_id) REFERENCES `user` (id),
	FOREIGN KEY (column_id) REFERENCES column (id)

[ffinn92]

오 넵! 가독성이 훨씬 좋아보이네요. 수정하겠습니다. 감사합니다. !

[blossun]

요청과 응답 DTO를 적절히 만들어주셨군요👍