아래부분은 HowHair 플젝을 하면서 백엔드 개발 팀원분들과 코드리뷰를 하며 매주 매주 api를 짰다.
혼자서 했다면 막막했을 것 같은데 같이 코드 리뷰를 하면서 많이 배웠다.
전체적으로 들었던 궁금증, 어려움
명명 규칙
이 리뷰 CRUD 작업을 하면서 첫 번째로 어려웠던 점은 메소드, 필드의 이름을 어떻게 짓느냐는 것이다.
좋은 이름은 짓는 것은 유지보수를 쉽게 한다.
- 축약하지 말자!(나는 알아도 다른 사람이 볼 때는 알기가 어려울 수도..)
- 이름을 보고 한번에 무슨 기능을 하는지 알 수 있도록 짓자.
- 메소드는 한가지의 일만을 하도록 하자.(이름을 짓기 어려운 것은 메소드가 여러가지 일을 해 어려운 것일 수도.. 명확한 한가지 일을 하는 메소드는 이름을 짓기도 쉽다)
클린 코드
코드를 짜면서도 내가 봤을때 효율적이지 않고 깨끗해보이지 않으며 설명할때도 어려움이 있었다. (쉬운 코드임에도) 가독성이 좋지 않은 코드라고 스스로도 느꼈다. 개발을 하면서 어떤게 좋은 코드일까를 많이 고민하게 되었다. 또한 아직 객체 지향 언어의 사용에 있어 부족함을 많이 느끼게 되는 것 같다. 객체지향언어와 클린코드에 대해 더 공부를 해야겠다.
클린한 코드를 짜면 유지보수하기가 쉽다.
한 메소드에 너무 많은 코드를 넣지 말고 명확한 알기쉬운 이름으로 메소드를 만들어 쪼개주자.
퍼사드 패턴
아래 코드와 같이 ReviewService안에 awsS3Service, reviewImageService, reviewRepository, memberRepository를 같이 주입 받아 사용하고 있다.
public class ReviewService {
private final ReviewRepository reviewRepository;
private final MemberRepository memberRepository;
private final AwsS3Service awsS3Service;
private final ReviewImageService reviewImageService;그러나 review에서도 reviewImage repository를 사용하고, reviewImage에서도 review repository를 사용하여 뭔가 짬뽕되는 느낌이 들었다.
이 부분은 퍼사드 패턴이라는 디자인 패턴으로 review와 reviewImage를 묶어 하나의 폴더에 넣어 해결하였다.
s3 시크릿 키 노출
application.yml에 적어두었던 aws s3 secreatkey들을 모르고 github에 올려버렸다. 며칠있다가 aws에서 메일이 왔는데 aws의 빠른 대응에 너무 신기했으며 다시는 노출하지 않도록 주의하자! 과도한 비용이 나올수 있다.
Intellij 유료/무료
webflex를 사용하여 카카오톡 로그인 api 사용자 정보를 받아오려고 했다. 하지만 자꾸 오류가 나며 실행이 되지 않아 몇시간 동안 오류를 해결하기 위해 끙끙거렸다. jar파일이 없어 클래스 로더가 불러오지 못함...뭐 어쩌고 저쩌고 인 것 같아 jar파일도 넣어보고 다 해봤는데도 고쳐지지 않아 눈물이 났던..
intellij무료 버전을 사용하고 있었는데 유료 버전을 사용해보라는 조언을 받고 다운 받아 실행해보았더니 실행이 너무 잘돼서 놀랐다.
역시 유료를 사용하는 이유가 있다.. 해결이 되어서 너무 다행이었다.
Review Controller에서
@RequiredArgsConstructor
@RestController
@RequestMapping("/review")
public class ReviewController {
private final ReviewService reviewService;
private final ReviewImageService reviewImageService;
@PostMapping("/new")
public ApiResponse<ReviewResponseDto> postReview(@RequestAttribute Long memberId,@Valid ReviewNewReqDto reviewRequestDto){
/**
* paramDto로 다시 셋하는게 맞나? 효율적인가?
* */
ReviewNewParamDto reviewNewParamDto = ReviewNewParamDto.builder()
.content(reviewRequestDto.getContent())
.date(reviewRequestDto.getDate())
.shopName(reviewRequestDto.getShopName())
.lengthStatus(reviewRequestDto.getLengthStatus())
.designerName(reviewRequestDto.getDesignerName())
.price(reviewRequestDto.getPrice())
.satisfaction(reviewRequestDto.getSatisfaction())
.dyeing(reviewRequestDto.getDyeing())
.hairCut(reviewRequestDto.getHairCut())
.perm(reviewRequestDto.getPerm())
.straightening(reviewRequestDto.getStraightening())
.imageFiles(reviewRequestDto.getImageFiles())
.build();
Long reviewId = reviewService.createReview(memberId, reviewNewParamDto);
return ApiResponse.success(ReviewResponseDto.builder().reviewId(reviewId).build());
}
리뷰 Controller에서 궁금했던 점!
paramDto로 다시 셋하는게 맞나? 효율적인가?
Controller에서 받는 DTO와 Service의 DTO를 구분해서 사용해주는 것이 좋다.
용도와 구분을 명확하게 하기 위해서.
ResquestDTO, ResponseDTO는 Controller에서만 사용하고 Service로 DTO를 넘겨줄때는 ParamDTO와 같게 넘겨주자.
위 코드와 같이 ParamDTO로 모든 값들을 설정해서 넘겨주는 것이 비효울적으로 보이고 귀찮더라도 변환을 해서 넘겨주자!
또한 Controller, Service, Repository에서 데이터를 넘겨줄때도 DTO를 사용하자.
Review Service에서
@Transactional
public Long createReview(Long memberId, ReviewNewParamDto reviewNewParamDto) {
//status 고려
Member member=getMember(memberId);
/**
* cut, perm, dyeing, straightening이 DTO로 들어올때부터 기본이 NONE으로 받는다고 가정
* */
Review review = Review.builder()
.content(reviewNewParamDto.getContent())
.date(reviewNewParamDto.getDate())
.status(ACTIVE)// 삭제하면 Status.INACTIVE로 변경
.designerName(reviewNewParamDto.getDesignerName())
.satisfaction(reviewNewParamDto.getSatisfaction())
.hairShopName(reviewNewParamDto.getShopName())
.price(reviewNewParamDto.getPrice())
.content(reviewNewParamDto.getContent())
.hairCut(reviewNewParamDto.getHairCut())
.dyeing(reviewNewParamDto.getDyeing())
.straightening(reviewNewParamDto.getStraightening())
.perm(reviewNewParamDto.getPerm())
.lengthStatus(reviewNewParamDto.getLengthStatus())
.member(member)
.build();
reviewRepository.save(review);
/**
* 리뷰에서 이미지를 받았다면
* **/
if(reviewNewParamDto.getImageFiles()!=null){
List<String> imgPaths = awsS3Service.upload(reviewNewParamDto.getImageFiles());
for (String imgUrl : imgPaths) {
ReviewImage img = ReviewImage.builder()
.url(imgUrl)
.status(ACTIVE)
.review(review)
.build();
reviewImageService.saveImage(img);
}
}
return review.getId();
}수정되어야 할 부분
리뷰를 생성할 때 현재는
Review entity 저장 -> S3에 이미지 저장 -> ReviewImage 저장
과정으로 진행된다.
여기서 중요한 점은 entity save fail이 일어날 수 있음을 고려하지 않았다는 점이다.
만약 파일이 먼저 저장되고 Entity가 save가 rollback이 되어 버리면 파일에 저장된 것은 쓰레기 파일이 되어 버린다.
따라서 entity와 파일의 저장 순서는
- Entity 저장 후
- 파일 저장
이 되어야 한다.
만약 이런 과정으로 했는데도 save가 fail이 되어서 롤백이 발생된다면, 현업에서는 그냥 나둔다고 한다.
파일을 건드는?(삭제하는 것과 같은) 일을 하면 IOException이 발생할 수 있기 때문이다.
그렇다면 궁금했던 점이 entity를 먼저 저장하고 파일을 저장하면 reviewImage entity를 어떻게 저장할까?였다. 로컬에 저장을 먼저하고 해야하나라는 궁금증이 생겼다.
로컬에는 저장을 하지 않고 이름만 먼저 만들고 저장을 하는 방식으로 했으며 그 후 s3에 저장을 했다.
Review Image 생성 과정에서
public List<String> upload(List<MultipartFile> multipartFile) {
List<String> imgUrlList = new ArrayList<>();
// forEach 구문을 통해 multipartFile로 넘어온 파일들 하나씩 fileNameList에 추가
for (MultipartFile file : multipartFile) {
String fileName = createFileName(file.getOriginalFilename());
ObjectMetadata objectMetadata = new ObjectMetadata();
objectMetadata.setContentLength(file.getSize());
objectMetadata.setContentType(file.getContentType());
try(InputStream inputStream = file.getInputStream()) {
s3Client.putObject(new PutObjectRequest(bucket+"/post/image", fileName, inputStream, objectMetadata)
.withCannedAcl(CannedAccessControlList.PublicRead));
imgUrlList.add(s3Client.getUrl(bucket+"/post/image", fileName).toString());
} catch(IOException e) {
throw new ApiException(ApiResponseStatus.WRONG_IMAGE,"이미지 업로드에 오류가 발생하였습니다.");
}
}
return imgUrlList;
}수정해야할 부분
s3에 저장을 할때 하나의 디렉토리만 생성하며 모든 이미지들을 때려 넣는 방식이며, 디렉토리를 나눠볼까라는 생각조차 하지 않았다. 하지만 이미지들을 확인하고자 할 경우 하나의 디렉토리에만 있으면 이게 어느 것의 이미지인지 구분하기가 어렵기 때문에 나누는 것이 좋다고 생각했다.
따라서 image path의 이름을 해당 날짜 + 어떤 리뷰ID인지로 확인하기 쉽게 변환을 해주었다.
// 이미지파일명 중복 방지
private String createFileName(String fileName) {
return UUID.randomUUID().toString().concat(getFileExtension(fileName));
}또한 현재 이미지파일명을 UUID를 이용하여 image이름을 생성해주었다.
이미지 URL이 사용자에게 넘어갈때 쉽게 식별할 수 있는 리소스를 담고 있으면 좋지 않다.
결론
2023-03-21
첫 플젝인데 코드리뷰를 하면서 많이 배울 수 있어서 너무 좋다.
하지만 공부해야할 것은 너무 많고,,,,,ㅠㅠ 개발자 되기 어렵다.
2023-04-04
현재도 꾸준히 개발하고 있으며, 코드를 계속 리팩토링 중이다. 백엔드를 강의를 보며 공부한지는 좀 됐는데 이게 첫 플젝이다. 그저 강의를 보는 것과 직접 만들면서 성장하는 부분이 확실히 달랐다. 이제서야 백엔드에 대해 처음 제대로 공부하는 느낌이다. 또한 봤던 강의인데 코드를 직접 짜다가 강의를 다시 보는 보면 다시 새롭게 느껴지고 좀 더 깊이있게 알아가게 되는 것 같다.
추가로
1. 리눅스에서는 path를 만들어 줄때 ,'/'를 넣으면 오류? 인식을 못한다고 한다. File.separator를 사용하면 좋다
2. 리뷰 이미지를 넣을 때 몇개까지 제한을 주도록 @Size annotation을 추가해주자
3. FileNameUtils.getExtension으로 확장자를 편하게 추출해줄 수 있다.
4. for문 사용을 지양하고 Stream을 지향하자!
5. 이미지 오류는 Handler에서 못잡기 때문에 IOException을 사용하여 try catch로 오류를 잡아주자!
6. CollectionUtils 라이브러리를 활용해보자
7. 배포 공부해보기(도커)
