[스터디] 클린 코드 스터디 1주차
클린 코드 스터디 1주차 : 1 ~ 3 장
1장 깨끗한 코드
[ C++창시자 비야네가 말하는 깨끗한 코드 ]
- 우아한 코드
보는사람에게 즐거움을 주는 코드 - 효율성있는 코드
코드자체의 속도나 메모리 뿐만이 아니라 나쁜코드를 작성해서 수정하는데 드는 비용을 줄여서 효율성을 챙겨야한다는 것
나쁜 코드는 나쁜코드를 유혹하고 낳는다
3. 철저한 오류 처리
세세한 사항까지 꼼꼼히 신경쓰라는 의미
4. 하나에 집중하라
너무 많은일을 하려고 애쓰다가 의도가 뒤섞이고 목적이 흐려진다.
[ 그래디 부치가 말하는 깨끗한 코드 : 가독성 ]
- 단순하고 직접적
- 설계자의 의도를 숨기지 않는다.
- 명쾌한 추상화와 단순한 제어문
코드는 추측이 아니라 사실에 기반해야한다.
반드시 필요한 내용만을 담아야한다.
[ 데이브토마스 말하는 깨끗한 코드 : 가독성 + 쉬운 수정 ]
읽기 쉬운 코드와 고치기 쉬운 코드는 다르다!
읽기 쉬운 것은 기본 + 고치기 쉬운 코드가 깨끗한 코드이다.
고치기 쉬운코드란 ?
작은 코드가 중요하다. 작은 단위의 테스트 케이스를 작성하라
그외…
- 주의를 기울여서 꼼꼼하게 작성하라
- 론 제프리스
- 모든 테스트를 통과한다.
- 중복이 없다.
- 시스템 내 모든 설계 아이디어를 표현한다.
- 클래스, 메서드, 함수 등을 최대한 줄인다.
- 예상한대로 작동하는 코드
- 작게 추상화 하라
[ 내가 생각해본 깨끗한 코드 ]
- 읽기 쉬운 코드
- 수정이 쉬운 코드
- 중복이 없는 코드
- 한 기능만을 수행하게 작성한 코드
- 직관적인 네이밍을 작성한 코드
2장 의미있는 이름
[ 의도를 분명히 밝혀라 ]
다음은 내가 프로젝트를 진행할 때 작성했던 코드이다.
public int findMyPrice(String token, long boardId) {
Board board = find(boardId);
Member member = memberService.findByAccessToken(token);
Optional<BoardMember> optionalBoardMember = boardMemberRepository.findByBoardAndMember(board, member);
if (optionalBoardMember.isEmpty()) {
return 0;
} else {
return optionalBoardMember.get().getMyPrice();
}
}
findMyPrice는 어떤 상품에 내가 입찰한 가격을 알려주는 메서드이다.
board는 어떤 게시글을 의미하는가? member는 어떤 멤버를 의미하는 거지? optionalBoardMember는?
의미를 분명히하여 다시 작성해보자
public int findMyPrice(String token, long boardId) {
Board board = find(boardId);
Member loginedMember = memberService.findByAccessToken(token);
Optional<BoardMember> myPriceInfo = boardMemberRepository.findByBoardAndMember(board, loginedMember);
if (myPriceInfo.isEmpty()) {
return 0;
} else {
return myPriceInfo.get().getMyPrice();
}
}
// 스터디원들의 추가 의견
// 이름은 그대로 하고 조건문을 옵셔널로 처리
// boardmember클래스의 이름을 아예 price로[ 그릇된 정보를 피하라 ]
- 널리 쓰이는 단어를 다른 뜻으로 사용하지 마라
- 실제 해당 자료형이 아니면 사용을 피해라 List자료형이 아니라면
멤버들의 집합이라고 memberList라고 사용하기 보다 memberGroup등 과 같이 사용하자 - 유사한 이름의 사용을 피하자
[ 의미있게 구분하라 ]
다음은 내가 알고리즘 문제를 풀 때 작성했던 코드이다.
문제를 빠르게 풀려다보니 변수명을 대충 작성했던 것 같다.
이것도 한 번 수정해보자.
// 큐브를 돌리는 메서드 udfblr은 돌리고 싶은 면 D는 시계방향, 반대방향을 뜻함
**public static void rotate(char udfblr, char D){
->>
public static void rotate(char face, char direction){**
**// 이중배열을 정렬하기위해 정의한 compare 함수
public int compare(int[] o1, int[] o2) {
->>
public int compare(int[] origin, int[] target) {**
[ 검색하기 쉬운 이름을 사용하라 ]
int[] reasonCnt = new int[5]; // 의도도 알 수 없고 검색을 5로 하기 어려움
->>
int weekDay = 5;
int[] reasonCnt = new int[weekDay];[ 인코딩을 피하라 ]
변수명에 타입을 선언해줄 필요가 없다
String nameString; // 굳이 string이라고 알려줄 필요가 없다.
->>
String name;접두어, 접미어의 사용도 줄이자 (인터페이스 클래스의 경우는 예외 ex impl)
[ 클래스 이름과 객체 이름은 명사 ]
메서드 이름은 동사**
접근 get~, 변경 set~, 조건 is~
[ 일관성있는 단어사용 ]
메서드를 set~로 했다가 update~로 했다 fetch~로 → X 의미가 같은 메서드라면 같은 단어를 사용하자
[ 의미있는 맥락을 추가하자 ]
String firstName;
String lastName;위 같은 변수가 있다면 이게 사람의 이름인지.. 주소의 이름인지
String addressFirstName;
String addressLastName;
String memberFirstName;
String memberLastName;명확히 맥락을 표현해주자 클래스를 따로 만들어주면 더 좋다.
3장 함수
[ 작게 만들어라 ]
내용이 길어지면 새로운 함수로 쪼개서 작게 더작게 .. if문 while문 등 에 들어가는 블록은 한줄로!
public void findPassword(MemberDto.Email dto) throws MessagingException {
String email = dto.getEmail();
if (memberRepository.countByEmail(email) == 0) {
throw new ResponseStatusException(ExceptionCode.MEMBER_NOT_FOUND.getCode(), ExceptionCode.MEMBER_NOT_FOUND.getMessage(), new IllegalArgumentException());
}
Member member = findByEmail(email);
String newPassword = randomPasswordService.genPassword();
emailService.findPassword(email, newPassword);
member.changePassword(passwordEncoder().encode(newPassword));
}내가 과거에 작성했던 비밀 번호 찾기 로직이다. 에러를 던지는 부분을 따로 클래스, 메서드화해서 분리해도 좋을 것 같고 이메일이 존재하지 않는 상황은 자주 발생하기 때문에 if문 자체를 하나의 메서드로 뽑아내도 좋을 것 같다.
[ 한가지만 해라 ]
한가지만 하라? 한가지는 무엇인가 위의 코드에서 예를 들면
- 이메일을 받아온다
- 이메일이 있는지 확인한다
- 이메일을 통해 멤버를 가져온다
- 랜덤 비밀번호를 생성한다
- 이메일로 임시 비밀번호를보내준다
- 멤버의 패스워드를 임시 비밀번호로 변경한다
이러면 한가지가 아니라 6가지를 하는건가? 아니다 추상화 수준이 하나인 단계만 수행하면 된다.
위 코드를 수정해보면
public void findPassword(MemberDto.Email dto) throws MessagingException {
memeber = getMemberByEmail(dto.getEmail());
String tempPassword = randomPasswordService.genPassword();
emailService.findPassword(email, tempPassword );
// 번외로 스터디원분이 emailService의 메서드이름도 변경하면 좋겠다는 의견
// -> emailService.sendTempPassword(email, tempPassword );
member.changePassword(passwordEncoder().encode(tempPassword ));
}
public Member getMemberByEmail(String email) {
return memberRepository.findByEmail(email)
.orElseThrow(
() -> new ResponseStatusException(ExceptionCode.MEMBER_NOT_FOUND.getCode(), ExceptionCode.MEMBER_NOT_FOUND.getMessage(), new IllegalArgumentException())
);
}하나의 메서드에 8줄이 무식하게 들어있는게 아니고 3~4줄로 줄였다.
( 리팩토링 하다보니 getMemberByEmail 메서드를 작성해놓고 다른 곳에서만 사용하고 있었다.. 메서드를 잘확인하고 사용하자.. )
[ 추상화 수준을 하나로 ]
예시를 보면 맨처음 메서드는 “이메일을 받아와서 존재하는 계정인지 확인 후 존재한다면 계정 정보를 가져와 임시 비밀번호를 생성, 저장하고 이메일로 임시 비밀번호를 보내준다.” 의 기능을 수행하게 작성했다.
이는 추상화 수준이 높은 코드이다. 하나의 메서드에 많은 기능이 들어갔다.
수정 후 코드는 “이메일을 통해 계정정보를 가져와 임시 비밀번호를 생성, 저장하고 이메일로 임시 비밀번호를 보내준다.” + “이메일을 통해 존재하는 계정인지 확인 후 계정정보를 가져온다.” 의 기능으로 나눠서 추상화의 수준을 낮췄다.
만약 이보다 더 낮은 수준을 구사하려면
“이메일을 통해 계정정보를가져온다” + “임시 비밀번호를 생성한다” + “임시 비밀번호를 이메일로 보내준다” + “임시 비밀번호를 저장한다” 뭐 이런 느낌이려나
이런 수준을 모든 메서드에 일정하게 유지해야한다는 것이다.
[ Switch 문 ]
스위치문을 사용하게되면 srp, ocp 등 객체지향의 원칙을 위반할 수 있다.
더 큰 문제는 이런 스위치문이 하나가 아니라 여러개 존재할 수도 있다는 점이다.
이를 예방하기 위해 추상클래스를 만들어 해결한다.
추상클래스에서 스위치문으로 나누고 각 클래스에서 이 추상클래스를 구현한다.
[ 서술적인 이름을 사용하라 ]
2장에서 얘기한 내용
[ 함수 인수는 적게 사용 ]
인수는 적을 수록 좋다. 인수가 많아지고 자주 사용된다면 별도 클래스를 만드는 방법도 생각해보자
[ 명령과 조회를 분리하라 ]
세팅하고 그 결과값을 리턴해주는 메서드는 지양하자.
public Board setBoard(){
...
boardRepository.save(board)
return board;
}분명 set이라고 해서 저장만하는 줄 알았는데?... 리턴값이있네?
명령메서드인가 조회 메서드인가?
[ 구조적 프로그래밍 ]
다익스트라의 구조적 프로그래밍 원칙에 따르면 함수에는 return문이 하나여야한다.
루프 안에서 break나 continue는 사용하지 않는게 좋다. 함수가 작다면 괜찮을 수도..