Skip to content

Check#8

Open
jaekkang wants to merge 28 commits intomainfrom
ProgramController
Open

Check#8
jaekkang wants to merge 28 commits intomainfrom
ProgramController

Conversation

@jaekkang
Copy link
Contributor

No description provided.

Copy link
Contributor

@honeyl3ee honeyl3ee left a comment

Choose a reason for hiding this comment

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

확인하시고 의견 주세요

}

@Override
public List<BannerDto> findAll() {
Copy link
Contributor

Choose a reason for hiding this comment

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

메서드 명 findBanners로 수정하면 더 명확할 것 같아요. 위에 findById도 마찬가지요.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

여러 데이터 가져오는 메서드는 findBanners 처럼 복수형 엔티티이름으로 끝나게 수정했습니다. 근데 findById는 괜찮을거 같아요.

ex) bannerService.findById() 로 되니까 그렇게 생각헀는데 어떠세요?

this.isOpened = true;
}

public void update(String title, String imgUrl) {
Copy link
Contributor

Choose a reason for hiding this comment

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

faq처럼 updateBanner 이런 식으로 메서드명 통일하면 좋겠습니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

faq.update() 처럼 되서 banner 도 updateBanner 가 아닌 그냥 update로 바꾸고 싶은데 어떠세요?

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