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

Feat: 즐겨찾기 등록 #6 #8

Merged
merged 5 commits into from
Nov 26, 2024
Merged

Feat: 즐겨찾기 등록 #6 #8

merged 5 commits into from
Nov 26, 2024

Conversation

Ivoryeee
Copy link
Collaborator

@Ivoryeee Ivoryeee commented Nov 25, 2024

✨ 해당 이슈 번호 ✨

closes #6

✨ todo ✨

  • Controller 구현
  • Service 구현
  • Repository 구현
  • Entity 구현

📌 내가 알게 된 부분

📌 질문할 부분

//BurgerService.java
    @Transactional
    public void updateLikeStatus(long burgerId) {
        BurgerEntity burgerEntity = burgerRepository.findById(burgerId)
                .orElseThrow(()-> new McdonaldException("버거 id를 찾을 수 없습니다.", HttpStatus.NOT_FOUND));
        burgerEntity.setLiked(!burgerEntity.getIsLiked());
        burgerRepository.save(burgerEntity);
        BurgerResponse.of(burgerEntity);
    }

논리 연산자 !를 사용해 좋아요 boolean 값을 수정하고 해당 값을 repository로 바로 저장되도록 했습니다.
로직이 간단하여 이와 같이 나타냈는데, 더 가독성 좋게 개선할 부분이 있을지 고민됐습니다.

image

@Ivoryeee Ivoryeee added the enhancement New feature or request label Nov 25, 2024
@Ivoryeee Ivoryeee requested a review from Ho-Tea November 25, 2024 16:06
@Ivoryeee Ivoryeee self-assigned this Nov 25, 2024
Copy link
Collaborator

@Ho-Tea Ho-Tea left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!
코멘트 확인해주시면 감사하겠습니닷


@Getter
@AllArgsConstructor
public class ApiResponse {
Copy link
Collaborator

Choose a reason for hiding this comment

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

사소하지만 record로 대체해도 될 것 같아용

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

동의합니다! 아래와 같이 record 로 수정해 두었습니다!

package com.sopt.mcdonald.burger.api.dto.response;

public record ApiResponse( String status, String message) {
}

BurgerEntity burgerEntity = burgerRepository.findById(burgerId)
.orElseThrow(()-> new McdonaldException("버거 id를 찾을 수 없습니다.", HttpStatus.NOT_FOUND));
burgerEntity.setLiked(!burgerEntity.getIsLiked());
burgerRepository.save(burgerEntity);
Copy link
Collaborator

Choose a reason for hiding this comment

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

JPA를 사용하게 되었을 때 디비에서 조회한 엔티티는 하나의 트랜잭션 내부에서 영속화 되어 관리되기때문에 따로 save를 호출해주지 않아도 변경사항들이 자동으로 저장됩니닷 말이 길었는데 결론은 아래와 같이 줄일 수 있을 것 같아요!

BurgerEntity burgerEntity = burgerRepository.findById(burgerId)
                .orElseThrow(()-> new McdonaldException("버거 id를 찾을 수 없습니다.", HttpStatus.NOT_FOUND));
burgerEntity.setLiked(!burgerEntity.getIsLiked());
// save 메서드 삭제
// BurgerResponse가 사용되는 부분이 없다고 판단해서 삭제

찾은 버거가 기존에 가지고 있던 값의 반대(true이면 false, false이면 true)로 구성되게 하셨으니까 set을 통해 외부에서 내부로 값을 직접 넣어주는 것보다 내부에서 자체적으로 값이 바뀌게 하는것이 조금 안정적일 것 같아요!
아래는 예시입니당 확인해보시고 선택적으로 반영해주세용

// BurgerService
burgerEntity.switchLiked();

// BurgherEntity
public void switchLiked() {
    this.isLiked = !this.isLiked;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오호 @transactional 사용 시 dirty Checking으로 변경이 감지된다는 게 이런 부분이군요! 덕분에 다시끔 인지할 수 있었어요 감사합니다 :)
말씀주신 대로 내부에서 값이 수정될 수 있게끔 하는게 바람직할 것 같네요.

// Burger Entity
    public void setLiked(boolean isLiked) {
        this.isLiked = isLiked;
    }

로 작성해 두었던 부분을

// BurgerEntity 
     public void switchLiked() { 
         this.isLiked = !this.isLiked; 
   }

요렇게 수정했습니다!

Copy link
Collaborator

@Ho-Tea Ho-Tea left a comment

Choose a reason for hiding this comment

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

고생하셨습니다아아 👍

@Ho-Tea Ho-Tea merged commit a317f5f into main Nov 26, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: 즐겨찾기 등록
2 participants