1. 객체의 상태를 변경한 후 출력하는 함수를 만들지 마라.
객체의 변경은 객체 내부에서만 하는 것이다. 다른 객체에게 변경이나 생성을 맡기려 하지마라.
public class MovieRepository {
private static List<Movie> movies = new ArrayList<>();
static {
Movie movie1 = new Movie(1, "생일", 8_000);
movie1.addPlaySchedule(new PlaySchedule(createDateTime("2019-04-16 12:00"), 6));
movie1.addPlaySchedule(new PlaySchedule(createDateTime("2019-04-16 14:40"), 6));
...이하 생략
movies.add(movie1);
}
영화 예매 목록을 담고 있는 객체이다.
Movie의 스케쥴을 추가하고 있는데, MovieRespository에서 추가하는 것이 아니라, Movie 내의 메소드에서 추가하고 있다.
public class Movie {
private final int id;
private final String name;
private final int price;
private List<PlaySchedule> playSchedules = new ArrayList<>();
void addPlaySchedule(PlaySchedule playSchedule) {
playSchedules.add(playSchedule);
}
}엄연히 스케쥴을 관리해야 하는 것은 변수를 가지고 있는 Movie의 역할이다.
2. get 메소드를 무작정 만들지 마라 set메소드는 아예 만들지 마라
public class BookMovie {
List<Movie> bookMovieList = new ArrayList<>();
int bookingPeople;
public BookMovie() {
}
public List<Movie> getBookMovieList() {
return bookMovieList;
}
public int getBookingPeople() {
return bookingPeople;
}
public void addBookMovieList(Movie movie) {
bookMovieList.add(movie);
}
public void setBookingPeople(int peopleNumber) {
this.bookingPeople = peopleNumber;
}이와 같이 코드를 작성한 건 결국 밖에서 따로 계산을 해주고 객체를 꺼내야 할 일이 많기 때문이다.
public class BookMovie {
private Movie bookMovie;
private int bookingPeople;
public BookMovie(Movie bookMovie, int bookingPeople) {
this.bookMovie = bookMovie;
this.bookingPeople = bookingPeople;
}해당 코드면 충분하다.
3. 굳이 밖에서 input을 받아서 파라미터로 넘기지 말자(파라미터를 쓸때없이 만들지 말자)
public static ReservedMovie reserve(){
int requestId = InputView.inputMovieId();
setReservedMovie(requestId);
selectSchedule();
return reservedMovie;
}
private static void setReservedMovie(int requestId) {
try {
findRequestMovie(requestId);
}catch(Exception e){
System.out.println(e);
reserve();
}
} public static ReservedMovie reserve(){
setReservedMovie();
selectSchedule();
return reservedMovie;
}
private static void setReservedMovie() {
try {
int requestId = InputView.inputMovieId();
findRequestMovie(requestId);
}catch(Exception e){
System.out.println(e);
setReservedMovie();
}
}requestId를 받는 위치만 다르지만, 굉장히 다르게 움직인다.
에러 발생시에 setReservedMovie를 리턴하는 것이 원래는 옳다.
하지만 파라미터를 밖에서 받게 되면 첫번째 코드처럼 처리하기 난감해진다.
항상 받을 값들을 인자로 넘기지 않는 방법을 생각 하자
이후 BookMovie를 생성하는 시스템을 수정하자.
BookSystem
일단 BookMovie의 구조부터 달라졌기 때문에 모든 함수를 손봐야 할 것 같다.
booking
1. 일단 이름이 동명사라서 거슬린다. 그렇다고 book으로 하기에는 명사 "책" 과 헷갈릴 수 있다고 생각 된다.
그래서 reserve로 모든 book들을 바꾸었다.
2. 이 함수는 그저 MovieRepository에서 영화가 있는지 확인 할 뿐이다. 그렇기 때문에 이름과 역할이 맞지 않다.
그래서 함수를 분리 시킨다.
3. setBookMovie는 더이상 리스트가 아니기 때문에, 바꾸어 주어야 한다. 클래스 변수로 willreserveMovie를 선언하여, 예약한 영화를 만들때 사용하게 한다.
public ReservedMovie reserve(int movieId) {
checkMovieExist(movieId);
selectPeopleBooking();
return reservedMovie;
}
private void checkMovieExist(int movieId){
try {
Validation.checkMovieId(movieId);
for (int i = 0; i < MovieRepository.getMovies().size(); i++) {
Movie movie = MovieRepository.getMovies().get(i);
setBookMovie(movie, movieId);
}
} catch (Exception e) {
reserve(InputView.inputMovieId());
}
}
private void setBookMovie(Movie movie, int movieId) {
if (movie.getId() == movieId) {
willreserveMovie = movie;
}
}
selectPeopleBooking
1. 외부 클래스에서 사용할 필요 없이 reserve에서 처리하게 할 수 있다. 그러므로 메소드를 숨긴다.
2. willreserveMovie 변수가 있기 때문에, 굳이 playSchedule을 새로 만들어 가져올 필요가 없다.
3. selectSchedule이 먼제 계산되어야 한다. 위치를 바꾼다.
private static int setReservePeopleNumber(){
try {
int requestPeopleNumber = InputView.inputReservePeopleNumber();
checkReserveCapacity(requestPeopleNumber);
return requestPeopleNumber;
}catch(Exception e){
System.out.println(e);
return setReservePeopleNumber();
}
}
private static void checkReserveCapacity(int requestPeopleNumber)throws Exception{
if(willreserveMovie.getReservedCapacity()<requestPeopleNumber){
throw new Exception();
}
}1. 예약된 수는 리턴값이 있는게 맞다! 왜냐하면 마지막에 ReservedMovie에 넣기 위해서 분명히 필요한 값이기 때문이다.
2. willreserveMovie의 함수로써 수용 가능한 인원에 대한 처리를 마쳤다.
willreserveMovie를 꺼내온 후 스케쥴을 꺼내고, 스케쥴 데이터 중 인원을 가지고 오는 것보다 훨씬 깔끔하다.
selectSchedule
일의 순서는 다음과 같다.
selectedScheduleList = reservedMovie.getBookMovieList().get(order).getPlaySchedules(); // 고른 영화의 스케줄을 담고
PlaySchedule selectedSchedule = selectedScheduleList.get(scheduleNumber - 1); // 그 스케줄 중 하나를 고르고
reservedMovie.getBookMovieList().get(order).getPlaySchedules().clear(); // 기존의 스케줄을 비운 후
reservedMovie.getBookMovieList().get(order).getPlaySchedules().add(selectedSchedule); // 하나를 추가한다.
하지만 계속 객체에서 값을 꺼내다 보니 복잡해졌다.
willreserveMovie 내부에서 처리하면 더욱 간단해 진다.
class ReserveSystem{
private static void selectSchedule(int requestScheduleNumber){
willreserveMovie.migrateReserveMoviebyMoive(requestScheduleNumber);
}
}public class Movie {
public void migrateReserveMoviebyMoive(int requestNumber){
PlaySchedule selectedSchedule = playSchedules.get(requestNumber-1);
initializeSchedule();
playSchedules.add(selectedSchedule);
}
private void initializeSchedule(){
playSchedules.clear();
}
}ReserveSystem에선 단지 ReserveMovie의 스케쥴을 migrate할 뿐이다!
결과
public class BookSystem {
private BookMovie bookMovie;
private List<PlaySchedule> selectedScheduleList;
public BookSystem(BookMovie bookMovie) {
this.bookMovie = new BookMovie();
}
public BookMovie booking(int movieId) {
try {
Validation.checkMovieId(movieId);
for (int i = 0; i < MovieRepository.getMovies().size(); i++) {
Movie willBookMovie = MovieRepository.getMovies().get(i);
bookMovie = setBookMovie(willBookMovie, movieId);
}
} catch (Exception e) {
booking(InputView.inputMovieId());
}
return bookMovie;
}
public void selectPeopleBooking(int peopleNumber) {
try {
int availableNumber = selectedScheduleList.get(0).getCapacity();
Validation.checkBookingPeopleNumber(availableNumber, peopleNumber);
bookMovie.setBookingPeople(peopleNumber);
} catch (Exception e) {
System.out.println("인원 수 오류");
selectPeopleBooking(InputView.inputPeopleBooking());
}
}
public void selectSchedule(int order, int scheduleNumber) {
try {
Validation.checkScheduleSize(order, scheduleNumber);
selectedScheduleList = bookMovie.getBookMovieList().get(order).getPlaySchedules();
PlaySchedule selectedSchedule = selectedScheduleList.get(scheduleNumber - 1);
bookMovie.getBookMovieList().get(order).getPlaySchedules().clear();
bookMovie.getBookMovieList().get(order).getPlaySchedules().add(selectedSchedule);
return;
} catch (Exception e) {
System.out.println("예약 불가능한 스케쥴을 입력하셨습니다.");
selectSchedule(order, InputView.inputMovieSchedule());
}
}
private BookMovie setBookMovie(Movie willBookMovie, int movieId) {
if (willBookMovie.getId() == movieId) {
bookMovie.addBookMovieList(willBookMovie);
}
return bookMovie;
}
}이전에는 공개 함수가 3가지 있었다. 그리고 외부에서 값을 받아 쓰기 때문에 파라미터로 넘기고 객체를 꺼낼 일이 많았다.
public class ReserveSystem {
private static ReservedMovie reservedMovie;
private static Movie willreserveMovie;
public static ReservedMovie reserve(){
setReservedMovie();
selectSchedule();
int reserveNumber = setReservePeopleNumber();
completeReserve(reserveNumber);
return reservedMovie;
}
private static void setReservedMovie() {
try {
int requestId = InputView.inputMovieId();
findRequestMovie(requestId);
}catch(Exception e){
System.out.println(e);
setReservedMovie();
}
}
private static void findRequestMovie(int requestId)throws Exception{
for(Movie movie : MovieRepository.getMovies()){
checkMovieExist(movie, requestId);
}
if(willreserveMovie == null){
throw new Exception();
}
}
private static void checkMovieExist(Movie movie, int requestId)throws Exception{
if(IsSameMovieIdAndRequestId(movie,requestId)){
willreserveMovie = movie;
return;
}
}
private static boolean IsSameMovieIdAndRequestId(Movie movie , int requestId){
return movie.getId() == requestId;
}
private static void selectSchedule(){
try {
int requestScheduleNumber = InputView.inputRequestScheduleNumber();
checkScheduleIndex(requestScheduleNumber);
willreserveMovie.migrateReserveMoviebyMoive(requestScheduleNumber);
}catch(Exception e){
System.out.println(e);
selectSchedule();
}
}
private static void checkScheduleIndex(int requestScheduleNumber)throws Exception{
if(willreserveMovie.getPlaySchedulesSize()<requestScheduleNumber){
throw new Exception();
}
}
private static int setReservePeopleNumber(){
try {
int requestPeopleNumber = InputView.inputReservePeopleNumber();
checkReserveCapacity(requestPeopleNumber);
return requestPeopleNumber;
}catch(Exception e){
System.out.println(e);
return setReservePeopleNumber();
}
}
private static void checkReserveCapacity(int requestPeopleNumber)throws Exception{
if(willreserveMovie.getReservedCapacity()<requestPeopleNumber){
throw new Exception();
}
}
private static void completeReserve(int reserveNumber){
reservedMovie = new ReservedMovie(willreserveMovie,reserveNumber);
}
이후에는 함수가 많이 늘었다. 하지만 모두 private한 함수이다. 뿐만 아니라 모두 클래스 메소드와 변수로 선언되어 있다. 객체를 불러와 값을 꺼내는 것도 없기 때문에, 주렁주렁 기차처럼 연결된 메소드도 없다. 객체를 리턴하는 것도 마지막 하나 뿐이다. 우리의 목표는 예약 영화 객체 하나를 만들어 내기만 할 뿐이기 때문이다.
PaymentSystem
결제 시스템이 끝나기 전까지 정보를 유지하기 위해서 인스턴스 생성을 목적으로 클래스를 생성한다.
countInitTotalPrice
private double countInitTotalPrice(BookMovie bookMovie){
totalPrice = bookMovie.getBookingPeople() * bookMovie.getBookMovieList().get(0).getprice();
return totalPrice;
}역시 객체에서 꺼내는 방법이 아닌 객체에서 계산한 결과를 출력하자.
ReservedMovie{
public double calculateReservedMoviePay(){
return reserveMovie.getPrice() * reservePeopleNumber;
}
}
PaymentSystem{
addPrice(reservedMovie.calculateReservedMoviePay());
}이 외에는 다 비슷한 로직이다.
이제 다음 포스팅에 이 로직과 이전 로직으로 리팩토링을 동시에 진행 하며, 생산성과 유지보수가 얼마나 달라지는지 비교해 보자.
'TDD, CleanCode' 카테고리의 다른 글
| [TDD]테스트 주도 개발 예제(Money) - KentBeck (0) | 2019.05.06 |
|---|---|
| 클린코더스 강의 리뷰 2. Function (0) | 2019.04.22 |
| 클린코더스 강의 리뷰 1. OOP (0) | 2019.04.22 |
| Clean Code - 우아한 테크 코스 코드 시험 리뷰(2) 기능 추가 편 (0) | 2019.04.20 |
| Clean Code(로버트 마틴) - 코드 작성 메뉴얼 (0) | 2019.04.17 |
