Chapter04

Chapter 04

Movie program

Overview

영화 예매 시스템을 데이터 중심으로 바라보면서 발생하는 문제점에 대해서 짚어보는 내용이다.

책에서 잘못됐다는 내용을 얘기해주지 않으면 뭐가 잘못된건지 모를 코드가 있어서 리팩토링을 진행하고 싶은 욕구가 상당히 많이 느껴진다.

이후 챕터들에서 개선된 코드로 변경되긴 하지만 이번 챕터 내에서 진행될 만한 부분들은 진행해볼 예정이다.

물론 이런 내용들은 책을 보고 이해하면 되고 내가 지금부터 하려는 건 이 repository readme에도 언급이 되어 있는 내용이지만

• Java와 C#의 비교
• (필요시) 리팩토링 및 재설계

를 진행할 것이다.

이번 챕터 역시 test program이 없으므로 test program을 만들 예정이며 unit test 역시 추가로 포함시켜보려 한다.

Code review

Movie.java and Movie.cs

Code

package org.eternity.movie.step02;

import org.eternity.money.Money;

import java.time.DayOfWeek;
import java.time.Duration;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.util.Arrays;
import java.util.List;

public class Movie {
    private String title;
    private Duration runningTime;
    private Money fee;
    private List<DiscountCondition> discountConditions;

    private MovieType movieType;
    private Money discountAmount;
    private double discountPercent;

    public Movie(String title, Duration runningTime, Money fee, double discountPercent, DiscountCondition... discountConditions) {
        this(MovieType.PERCENT_DISCOUNT, title, runningTime, fee, Money.ZERO, discountPercent, discountConditions);
    }

    public Movie(String title, Duration runningTime, Money fee, Money discountAmount, DiscountCondition... discountConditions) {
        this(MovieType.AMOUNT_DISCOUNT, title, runningTime, fee, discountAmount, 0, discountConditions);
    }

    public Movie(String title, Duration runningTime, Money fee) {
        this(MovieType.NONE_DISCOUNT, title, runningTime, fee, Money.ZERO, 0);
    }

    private Movie(MovieType movieType, String title, Duration runningTime, Money fee, Money discountAmount, double discountPercent,
                  DiscountCondition... discountConditions) {
        this.movieType = movieType;
        this.title = title;
        this.runningTime = runningTime;
        this.fee = fee;
        this.discountAmount = discountAmount;
        this.discountPercent = discountPercent;
        this.discountConditions = Arrays.asList(discountConditions);
    }

    public MovieType getMovieType() {
        return movieType;
    }

    public Money calculateAmountDiscountedFee() {
        if (movieType != MovieType.AMOUNT_DISCOUNT) {
            throw new IllegalArgumentException();
        }

        return fee.minus(discountAmount);
    }

    public Money calculatePercentDiscountedFee() {
        if (movieType != MovieType.PERCENT_DISCOUNT) {
            throw new IllegalArgumentException();
        }

        return fee.minus(fee.times(discountPercent));
    }

    public Money calculateNoneDiscountedFee() {
        if (movieType != MovieType.NONE_DISCOUNT) {
            throw new IllegalArgumentException();
        }

        return fee;
    }

    public boolean isDiscountable(LocalDateTime whenScreened, int sequence) {
        for(DiscountCondition condition : discountConditions) {
            if (condition.getType() == DiscountConditionType.PERIOD) {
                if (condition.isDiscountable(whenScreened.getDayOfWeek(), whenScreened.toLocalTime())) {
                    return true;
                }
            } else {
                if (condition.isDiscountable(sequence)) {
                    return true;
                }
            }
        }

        return false;
    }
}

using System;
using System.Collections.ObjectModel;

public class Movie {
    private string title;
    private TimeSpan runningTime;
    public Money Fee { private set; get; }
    public ReadOnlyCollection<DiscountCondition> DiscountConditions { private set; get;}

    public MovieType MovieType { private set; get; }
    public Money DiscountAmount { private set; get; }
    public double DiscountPercent { private set; get; }

    public Movie(string title, TimeSpan runningTime, Money fee, double discountPercent, params DiscountCondition[] discountConditions)
        : this(MovieType.PERCENT_DISCOUNT, title, runningTime, fee, Money.ZERO, discountPercent, discountConditions)
    {
    }

    public Movie(string title, TimeSpan runningTime, Money fee, Money discountAmount, params DiscountCondition[] discountConditions)
        : this(MovieType.AMOUNT_DISCOUNT, title, runningTime, fee, discountAmount, 0, discountConditions)
    {    
    }

    public Movie(string title, TimeSpan runningTime, Money fee)
        : this(MovieType.NONE_DISCOUNT, title, runningTime, fee, Money.ZERO, 0)
    {    
    }

    private Movie(MovieType movieType, string title, TimeSpan runningTime, Money fee, Money discountAmount, double discountPercent, params DiscountCondition[] discountConditions)
    {
        this.MovieType = movieType;
        this.title = title;
        this.runningTime = runningTime;
        this.Fee = fee;
        this.DiscountAmount = discountAmount;
        this.DiscountPercent = discountPercent;
        this.DiscountConditions = Array.AsReadOnly(discountConditions);
    }

    public Money CalculateAmountDiscountedFee => MovieType == MovieType.AMOUNT_DISCOUNT ? Fee - DiscountAmount : Money.ZERO;

    public Money CalculatePercentDiscountedFee => MovieType == MovieType.PERCENT_DISCOUNT ? Fee - (Fee * DiscountPercent) : Money.ZERO;

    public Money CalculateNoneDiscountedFee => MovieType == MovieType.NONE_DISCOUNT ? Fee : Money.ZERO;

    public bool IsDiscountable(DateTime whenScreened, int sequence)
    {
        foreach (DiscountCondition condition in DiscountConditions)
        {
            if (condition.Type == DiscountConditionType.PERIOD && condition.IsDiscountable(whenScreened.DayOfWeek, whenScreened))
            {
                return true;
            }
            else
            {
                if (condition.IsDiscountable(sequence))
                {
                    return true;
                }
            }
        }

        return false;
    }
}

Getter,Setter vs Property

Movie.java를 Movie.cs로 바꾸면서 느낄 수 있는 가장 큰 변화는 역시 getter, setter를 한번에 묶어주는 C#의 property 문법이다.

Java의 이런 문법은 Lombok을 쓰고 싶게 만드는 매우 흔한 getter, setter 문법이다.

추가로 04 자율적인 객체 내용에서 설계 변경이 일어나는데, 느낌상 어쩔 수 없이 Movie에도 DiscountCondition에 따라 메서드가 추가된 느낌이 강하다. 파라미터도 그대로 받고 foreach, if문으로 이어지는 코드는 진짜 어쩔 수 없이 따라가는 코드라는 느낌만 잔뜩 준다.

각 MovieType에 따른 DiscountFee 계산 하는 부분은 DiscountCondition과 마찬가지로 예외 처리를 하지 않고 할인이 적용되면 해당 할인 적용을 그렇지 않으면 Money.ZERO를 통해 할인이 안된다는 방식의 ternary operator(삼항연산자)를 사용했다.

private Money fee;

public Money getFee() {
    return fee;
}

public void setFee(Money fee) {
    this.fee = fee;
}

Lombok을 쓰면 아래와 같이 쓸 수 있을 것이다.

@Getter
@Setter
public class Movie {
    private Money fee;
}

하지만 C#은 lombok과 같은 외부 패키지 없이 단 한줄로 이 코드를 구현할 수 있다. C# 모르는 사람이 보면 Fee라는 field 값을 public으로 선언하다니? 객체지향적이지 못하다! 라고 주장할 수 있으나 set; get; 이라는 키워드 안에는 setter, getter method가 구현이 되어 있으므로 객체지향의 본질인 캡슐화를 깨뜨리지 않는 아름다운 문법임을 알아야 한다.

public Money Fee { set; get; }

Collections.unmodifiableList vs ReadOnlyCollection

Java의 unmodifiableList는 이름 자체만으로도 매우 java스러운 메소드인데, 이름에서 추측 가능하듯이 list를 수정 불가능한 객체로 반환해 준다. 이 넘겨받은 객체는 read-only 형태의 객체로 감싸서 주는데 add 메소드라도 부르게 되면 exception을 발생시키 때문에 원본 list 객체 대신 reference list 객체를 주고 싶을 때 사용한다.

C#은 ReadOnlyCollection이라는 type이 있고 만약 이를 변환하는 코드를 짠다면 아래와 같은 코드로 짤 수 있다. 마찬가지로 add 메소드나 assign이라도 하려고 하면 exeption이 발생한다.

DiscountCondition[] discountConditions;
this.DiscountConditions = Array.AsReadOnly(discountConditions);

Call constructor from other constructor

Constructor(생성자) 코드가 overloading 되어 있다면 서로 호출해서 코드의 중복을 막을 수 있다.

여기서 java와 c#의 미묘한 문법의 차이가 존재하는데

• Java는 constructor body에 this overload constructor를 사용
• C#은 : 키워드를 사용해서 body에 진입하기 전에 this overload constructor를 호출

코드로 살펴 보면

public Movie(String title, Duration runningTime, Money fee, double discountPercent, DiscountCondition... discountConditions) {
    this(MovieType.PERCENT_DISCOUNT, title, runningTime, fee, Money.ZERO, discountPercent, discountConditions);
}

public Movie(string title, TimeSpan runningTime, Money fee, double discountPercent, params DiscountCondition[] discountConditions)
    : this(MovieType.PERCENT_DISCOUNT, title, runningTime, fee, Money.ZERO, discountPercent, discountConditions)
{
}

그런데 this overload constructor 호출 타이밍을 제어할 수 있다는 점에서 봤을 때 java 쪽이 조금 더 유리한 코드를 작성할 수 있다.

C#은 body에 진입하기 전에 this overload constructor call을 하기 때문에 미리 손 쓸 기회도 없이 시작하기 때문이다.

DiscountCondition.java and DiscountCondition.cs

Code

package org.eternity.movie.step02;

import java.time.DayOfWeek;
import java.time.LocalTime;

public class DiscountCondition {
    private DiscountConditionType type;

    private int sequence;

    private DayOfWeek dayOfWeek;
    private LocalTime startTime;
    private LocalTime endTime;

    public DiscountCondition(int sequence){
        this.type = DiscountConditionType.SEQUENCE;
        this.sequence = sequence;
    }

    public DiscountCondition(DayOfWeek dayOfWeek, LocalTime startTime, LocalTime endTime){
        this.type = DiscountConditionType.PERIOD;
        this.dayOfWeek= dayOfWeek;
        this.startTime = startTime;
        this.endTime = endTime;
    }

    public DiscountConditionType getType() {
        return type;
    }

    public boolean isDiscountable(DayOfWeek dayOfWeek, LocalTime time) {
        if (type != DiscountConditionType.PERIOD) {
            throw new IllegalArgumentException();
        }

        return this.dayOfWeek.equals(dayOfWeek) &&
                this.startTime.compareTo(time) <= 0 &&
                this.endTime.compareTo(time) >= 0;
    }

    public boolean isDiscountable(int sequence) {
        if (type != DiscountConditionType.SEQUENCE) {
            throw new IllegalArgumentException();
        }

        return this.sequence == sequence;
    }
}

using System;

public class DiscountCondition
{
    public DiscountConditionType Type { set; get; }
    public int Sequence { set; get; }
    public DayOfWeek DayOfWeek { set; get; }
    public TimeSpan StartTime { set; get; }
    public TimeSpan EndTime { set; get; }

    public bool IsDiscountable(DayOfWeek dayOfWeek, DateTime time) => Type == DiscountConditionType.PERIOD ? DayOfWeek == dayOfWeek && StartTime < time && EndTime > time : false;
    public bool IsDiscountable(int sequence) => Type == DiscountConditionType.SEQUENCE ? Sequence == sequence : false;
}

DiscountCondition은 C#의 Property로 그대로 바꾸기만 했다.

04 자율적인 객체 내용에서 설계 변경이 일어나는데, 나름 IsDiscountable() overload 메서드를 준비했지만 솔직히 이렇게 만들면 안된다는게 느껴질 정도다.

두 메서드는 특별히 C#으로 바꾸면서 리팩토링을 진행했는데 다음과 같다.

• Type이 PERIOD, SEQUENCE 2개 밖에 없는데, 굳이 exception으로 던질 이유가 없으므로 true condtion으로 변경 (java 코드는 IlligalArgumentExcpetion을 던진다)
• return 되어야 하는 값이 bool 이므로 true condition이 아니면 자연스럽게 false로 처리
• ternary operator(삼항연산자)를 사용해서 한줄로 코딩

Screening.java and Screening.cs

Code

package org.eternity.movie.step02;

import org.eternity.money.Money;

import java.time.LocalDateTime;

public class Screening {
    private Movie movie;
    private int sequence;
    private LocalDateTime whenScreened;

    public Screening(Movie movie, int sequence, LocalDateTime whenScreened) {
        this.movie = movie;
        this.sequence = sequence;
        this.whenScreened = whenScreened;
    }

    public Money calculateFee(int audienceCount) {
        switch (movie.getMovieType()) {
            case AMOUNT_DISCOUNT:
                if (movie.isDiscountable(whenScreened, sequence)) {
                    return movie.calculateAmountDiscountedFee().times(audienceCount);
                }
                break;
            case PERCENT_DISCOUNT:
                if (movie.isDiscountable(whenScreened, sequence)) {
                    return movie.calculatePercentDiscountedFee().times(audienceCount);
                }
            case NONE_DISCOUNT:
                movie.calculateNoneDiscountedFee().times(audienceCount);
        }

        return movie.calculateNoneDiscountedFee().times(audienceCount);
    }
}

using System;

public class Screening
{
    public Movie movie { set; get; }
    public int sequence { set; get; }
    public DateTime whenScreened { set; get; }

    public Money CalculateFee(int audienceCount)
    {
        Money money = Movie.CalculateNoneDiscountedFee;
        if (Movie.IsDiscountable(WhenScreened, Sequence))
        {
            if (Movie.MovieType == MovieType.AMOUNT_DISCOUNT)
            {
                money = Movie.CalculateAmountDiscountedFee;
            }
            else if (Movie.MovieType == MovieType.PERCENT_DISCOUNT)
            {
                money = Movie.CalculatePercentDiscountedFee;
            }
        }

        return money * audienceCount;
    }
}

Screening 역시 Property로 변경한 것 밖에 없다. 왠지 망해가는 class 설계라는게 눈에 보이기 시작한다.

04 자율적인 객체 내용에서 설계 변경이 일어나는데, 뭔가 개선이 되는 것처럼 보이지만 책 마지막에 설명한 것과 같이 끝은 아닌 그런 시원치 않은 개선 정도다.

Java의 원래 메서드와 다르게 리팩토링 한 점은 아래와 같다.

• switch if 문의 순서로 계산한게 아니라, if switch 문의 순서로 계산
  • 이유는 movie type 중에서는 NONE_DISCOUNT의 경우 따로 계산할 필요가 없기 떄문이다.
  • Movie.IsDiscountable() 메서드가 중복 if 문이라는게 눈에 너무 띄기 때문에라도 순서를 바꾸게 됐다.
• 매번 audienceCount를 계산하는데, 미리 discount fee가 계산이 되면 그 이후에 한번에 해도 무방한 곱셈이라서 그렇다.
• discount type이 결정되자 마자 return이 되는 방식이 아니라 CalculateNoneDiscountedFee 값의 경우 default 라고 생각해 보면 fee 계산이 끝난 이후 최종적으로 return 되는 방식으로 진행

Reservation.java and Reservation.cs

Code

package org.eternity.movie.step01;

import org.eternity.money.Money;

public class Reservation {
    private Customer customer;
    private Screening screening;
    private Money fee;
    private int audienceCount;

    public Reservation(Customer customer, Screening screening, Money fee,
                       int audienceCount) {
        this.customer = customer;
        this.screening = screening;
        this.fee = fee;
        this.audienceCount = audienceCount;
    }

    public Customer getCustomer() {
        return customer;
    }

    public void setCustomer(Customer customer) {
        this.customer = customer;
    }

    public Screening getScreening() {
        return screening;
    }

    public void setScreening(Screening screening) {
        this.screening = screening;
    }

    public Money getFee() {
        return fee;
    }

    public void setFee(Money fee) {
        this.fee = fee;
    }

    public int getAudienceCount() {
        return audienceCount;
    }

    public void setAudienceCount(int audienceCount) {
        this.audienceCount = audienceCount;
    }
}

using System;

public class Reservation
{
    public Customer Customer { set; get; }
    public Screening Screening { set; get; }
    public Money Fee { set; get; }
    public int AudienceCount { set; get; }

    public Reservation(Customer customer, Screening screening, Money fee,
                       int audienceCount) {
        Customer = customer;
        Screening = screening;
        Fee = fee;
        AudienceCount = audienceCount;
    }
}

Reservation은 생성자가 추가된거 빼고는 역시 망해가는 class 설계로 가고 있다.

Customer.java and Customer.cs

Code

package org.eternity.movie.step01;

public class Customer {
    private String name;
    private String id;

    public Customer(String name, String id) {
        this.id = id;
        this.name = name;
    }
}

public class Customer
{
    private string name;
    private string id;

    public Customer(string name, string id)
    {
        this.name = name;
        this.id = id;
    }
}

Customer는 그냥 보여주기식으로 만들었는데 이유는 private field 값 name, id에 constructor로 세팅하는 거 빼고는 하는게 없는 class이기 떄문이다. 그래서 string type에 대해 처음에 대문자냐 소문자냐만 다를 뿐 코드가 똑같다.

ReservationAgency.java and ReservationAgency.cs

Code

package org.eternity.movie.step02;

import org.eternity.money.Money;

public class ReservationAgency {
    public Reservation reserve(Screening screening, Customer customer, int audienceCount) {
        Money fee = screening.calculateFee(audienceCount);
        return new Reservation(customer, screening, fee, audienceCount);
    }
}

public class ReservationAgency
{
    public Reservation Reserve(Screening screening, Customer customer, int audienceCount) => new Reservation(customer, screening, screening.CalculateFee(audienceCount), audienceCount);
}

04 자율적인 객체 내용에서 설계 변경이 일어나는데, ReservationAgency class는 상당히 단순한 구조로 변하고 모든 Reservation에 필요한 건 screening에 맡기는 구조로 변경했다.

책에서도 언급하지만 사실 이정도 구조 변경도 꽤나 쓸모 있을 수 있지만 문제는 아직 있는 것 맞다.