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

[MODIFY] 홈 화면 인기레큐북 기준 수정 #73

Merged
merged 5 commits into from
Mar 10, 2024

Conversation

eeddiinn
Copy link
Contributor

@eeddiinn eeddiinn commented Mar 9, 2024

📌 관련 이슈

closed #72

✨ 어떤 이유로 변경된 내용인지

  • 인기 레큐북 기준을 노트가 많이 붙여진 레큐북 순으로 상위 6개로 설정한다.
  • 상위 6개를 하되, 만약에 6개를 못채우면 그 전달꺼로 채울 수 있도록 한다.
  • 최근 3달까지만 검사하고, 없으면 6개를 안채우도록 한다.
  • 인기 레큐북에 중복된 레큐북은 없도록 한다.

🙏 검토 혹은 리뷰어에게 남기고 싶은 말

  • 일단은 스케줄러 활용은 안하고 1안(단순 구현)만 해씁니다 ........................... 😢😢😢

@eeddiinn eeddiinn added 🪄API 서버 API 통신 예진❄️ 🔧Modify 코드 수정 (기능의 변화가 있을 때) 🔥 Pull Request PR 날림 labels Mar 9, 2024
@eeddiinn eeddiinn self-assigned this Mar 9, 2024
Copy link
Member

@ddongseop ddongseop left a comment

Choose a reason for hiding this comment

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

수고하셨습니다-! 회의 했던 내용대로 잘 구현해주신 것 같아요~
포스트맨으로 테스트 했을때도 잘 작동하네요!

@@ -28,15 +28,15 @@ public void init() {
}

private static final List<String> CHARACTER_STICKERS = Arrays.asList(
"https://dzfv99wxq6tx0.cloudfront.net/stickers/sticker_foryou.svg",
Copy link
Member

Choose a reason for hiding this comment

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

Intellij 기본 인덴트 조건이 다른 것 같습니다 ㅠ
https://bbubbush.tistory.com/30
탭 크기 2->4로 변경 부탁드려요!

@@ -59,18 +57,17 @@ public void addPostedSticker(PostedSticker postedSticker) {
}

@Builder
public Book(String uuid, String favoriteName, String favoriteImage, String title, String description, String backgroundColor, Member member, int popularRate) {
public Book(String uuid, String favoriteName, String favoriteImage, String title, String description, String backgroundColor, Member member) {
Copy link
Member

Choose a reason for hiding this comment

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

생성자 수정 잘 해주셨네요!

import java.util.List;
import java.util.Optional;

public interface BookRepository extends JpaRepository<Book, Long> {

@Query(value = "SELECT b FROM Book b WHERE b.popularRate > 0 ORDER BY b.popularRate ASC")
List<Book> findAllOrderByPopular();
@Query("SELECT DISTINCT b FROM Book b JOIN FETCH b.notes n WHERE n.createdAt >= :startDate AND n.createdAt <= :endDate")
Copy link
Member

Choose a reason for hiding this comment

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

쿼리문 잘 작성하셨네요..!!

} // 인기북이 6개 이상이 되거나 최근 3달을 다 검사했으면 for문을 빠져나옴
}

return popularBooks;
Copy link
Member

Choose a reason for hiding this comment

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

어려울 수 있는 로직인데, 주석과 함께 잘 작성해주셔서 잘 이해했습니다~

Copy link
Contributor

@dong2ast dong2ast left a comment

Choose a reason for hiding this comment

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

전반적으로 잘 구현하셨네여 👍 👍
앞으로의 행보 응원하겠습니다 ㅋㅋㅋㅋ

popularBooks.addAll(duplicateBook((books.subList(0, remain)), popularBooks));
} // 예를 들어 인기북에 2개가 있으면 4개만 더 추가하면 되니까 가져온 북의 인덱스 0부터 3까지만 가져오도록 함

if (popularBooks.size() >= 6 || i == 2) {
Copy link
Contributor

Choose a reason for hiding this comment

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

[P2] i 조건에 대한 검사는 기존 반복문에 포함되어 있어서 추가로 검사 안해도 좋을 것 같아요 :)

Comment on lines 58 to 64
popularBooks.addAll(duplicateBook(books, popularBooks));
} else {
int remain = 6 - popularBooks.size();
popularBooks.addAll(duplicateBook((books.subList(0, remain)), popularBooks));
} // 예를 들어 인기북에 2개가 있으면 4개만 더 추가하면 되니까 가져온 북의 인덱스 0부터 3까지만 가져오도록 함

if (popularBooks.size() >= 6 || i == 2) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
popularBooks.addAll(duplicateBook(books, popularBooks));
} else {
int remain = 6 - popularBooks.size();
popularBooks.addAll(duplicateBook((books.subList(0, remain)), popularBooks));
} // 예를 들어 인기북에 2개가 있으면 4개만 더 추가하면 되니까 가져온 북의 인덱스 0부터 3까지만 가져오도록 함
if (popularBooks.size() >= 6 || i == 2) {
popularBooks.addAll(duplicateBook(books, popularBooks));
continue;
}
int remain = 6 - popularBooks.size();
popularBooks.addAll(duplicateBook((books.subList(0, remain)), popularBooks));
break;
// 예를 들어 인기북에 2개가 있으면 4개만 더 추가하면 되니까 가져온 북의 인덱스 0부터 3까지만 가져오도록 함

[P2] 클린 코드를 지향하기 위해 else 예약어는 가급적 안쓰는 편이 좋습니다!
이에 더하여 가져온 레큐 북이 6개가 넘어가는 case는 이 반복문 내에서 한번만 발생하는 상황이니 break문을 통해 분기처리를 하면 조건문이나 코드 스타일 부분에서 최적화가 가능할 것 같습니다 ~~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오호 좋은 조언 감사함당 !

Comment on lines +38 to +55
public List<Book> sortBooksDesc(LocalDateTime startDate, LocalDateTime endDate) {
return bookRepository.findBooksMonth(startDate, endDate)
.stream()
.sorted(Comparator.comparingInt(book -> -book.getNotes().size())) // -를 붙여 내림차순 정렬
.collect(Collectors.toList());
}

// 최근 3달을 확인하는데 최근 순으로 먼저 인기레큐북에 추가할 수 있는 로직
// 현재를 0이라 하면 -1 ~ 0(최근 한달), -2 ~ -1(지지난달 부터 지난달), -3 ~ -2(지지지난달 부터 지지난달)
public List<Book> getPopularBooks() {
List<Book> popularBooks = new ArrayList<>();
LocalDateTime today = LocalDateTime.now();

// 최근 3달 확인
for (int i = 0; i < 3; i++) {
LocalDateTime endDate = today.minusMonths(i);
LocalDateTime startDate = endDate.minusMonths(1);
List<Book> books = sortBooksDesc(startDate, endDate);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public List<Book> sortBooksDesc(LocalDateTime startDate, LocalDateTime endDate) {
return bookRepository.findBooksMonth(startDate, endDate)
.stream()
.sorted(Comparator.comparingInt(book -> -book.getNotes().size())) // -를 붙여 내림차순 정렬
.collect(Collectors.toList());
}
// 최근 3달을 확인하는데 최근 순으로 먼저 인기레큐북에 추가할 수 있는 로직
// 현재를 0이라 하면 -1 ~ 0(최근 한달), -2 ~ -1(지지난달 부터 지난달), -3 ~ -2(지지지난달 부터 지지난달)
public List<Book> getPopularBooks() {
List<Book> popularBooks = new ArrayList<>();
LocalDateTime today = LocalDateTime.now();
// 최근 3달 확인
for (int i = 0; i < 3; i++) {
LocalDateTime endDate = today.minusMonths(i);
LocalDateTime startDate = endDate.minusMonths(1);
List<Book> books = sortBooksDesc(startDate, endDate);
public List<Book> sortBooksDesc(LocalDateTime today, int i) {
return bookRepository.findBooksMonth(today.minusMonths(i+1), today.minusMonths(i))
.stream()
.sorted(Comparator.comparingInt(book -> -book.getNotes().size())) // -를 붙여 내림차순 정렬
.collect(Collectors.toList());
}
// 최근 3달을 확인하는데 최근 순으로 먼저 인기레큐북에 추가할 수 있는 로직
// 현재를 0이라 하면 -1 ~ 0(최근 한달), -2 ~ -1(지지난달 부터 지난달), -3 ~ -2(지지지난달 부터 지지난달)
public List<Book> getPopularBooks() {
List<Book> popularBooks = new ArrayList<>();
// 최근 3달 확인
for (int i = 0; i < 3; i++) {
List<Book> books = sortBooksDesc(LocalDateTime.now(), i);

[P3] 이거는 그냥 지저분한 필드 값을 메서드 내로 옮겨버리면 이런 형태도 나올 수 있겠다 싶어서 첨언합니다!
보고 이런 스타일 맘에 드시면 적용하면 되구, 아니면 넘어가도 좋습니다 ㅎ ㅎ

Comment on lines 74 to 81
List<Book> duplicatedBook = new ArrayList<>();

for (Book book : books) {
if (!popularBooks.contains(book)) {
duplicatedBook.add(book);
}
}
return duplicatedBook;
Copy link
Contributor

@dong2ast dong2ast Mar 10, 2024

Choose a reason for hiding this comment

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

Suggested change
List<Book> duplicatedBook = new ArrayList<>();
for (Book book : books) {
if (!popularBooks.contains(book)) {
duplicatedBook.add(book);
}
}
return duplicatedBook;
return books.stream().filter(o -> popularBooks.stream()
.noneMatch(Predicate.isEqual(o))).collect(Collectors.toList());

[P3] 동일한 객체끼리 비교하는 로직이면 stream으로 위와 같이 작성할 수도 있어요!
이렇게 하면 popularBooks에 중복되지 않는 book 리스트만 남아서 return됩니다!
이것도 위에 것처럼 참고로 봐주세요~~

(예시)
List oldList = Arrays.asList("1", "2", "3", "4");
List newList = Arrays.asList("3", "4", "5", "6");

	List<String> oldNoneMatchList = oldList.stream().filter(o -> newList.stream()
			.noneMatch(Predicate.isEqual(o))).collect(Collectors.toList());

	List<String> newNoneMatchList = newList.stream().filter(n -> oldList.stream()
			.noneMatch(Predicate.isEqual(n))).collect(Collectors.toList());

	System.out.println(oldNoneMatchList.toString());		// [1,2]
	System.out.println(newNoneMatchList.toString());		// [5,6]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍👍👍🔥

@eeddiinn eeddiinn merged commit 3e7c917 into develop Mar 10, 2024
1 check passed
@eeddiinn eeddiinn deleted the modify/#72-home_modify branch March 10, 2024 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
예진❄️ 🪄API 서버 API 통신 🔧Modify 코드 수정 (기능의 변화가 있을 때) 🔥 Pull Request PR 날림
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[MODIFY] 홈 화면 인기레큐북 기준 수정
3 participants