5주차. 클린코드와 리팩토링 (1)

클린코드와 리팩토링

클린코드란?

유연하고, 견고하며, 유지보수하기 쉬운 코드를 클린코드라고 한다. 협업에서 다른 개발자들이 내 코드를 읽었을 때, 그 의도를 쉽게 파악하고, 수정 및 기능을 추가할 수 있어야한다.

리팩토링이란?

클린코드를 만들어가는 활동이자, 코드의 구조를 개선하는 활동이다. 소프트웨어의 겉으로 보이는 동작은 유지하고, 그 내부 구조들을 변경해서 이해가 쉽고 수정하기 쉽게 만드는 과정을 리팩토링이라고한다. 버그를 잡거나 새로운 기능을 추가하는 것은 리팩토링에 포함되지 않는다.

클린코드의 목표

좋은 코드라는게 뭔가 이상적이고 좋은 말이지만, 실제로 뭐가 좋은 코드인지는 감이 잘 안올 수도 있는 것 같다. 수업 내에서 나쁜 코드와 그 코드를 어떻게 해야 좋은 코드라고 할 수 있는건지 표로 비교해서 보는 시간을 가졌다.

나쁜 코드 스멜	좋은 코드 향기
거대클래스: 하나의 클래스가 너무 많은 책임과 코드를 가지고 있어서 비대해진 상태를 말한다.	단일책임클래스(SRP): 클래스가 하나의 명확한 책임을 가지며, 작고 응집도가 높은 것을 말한다.
긴 메소드: 하나의 메소드가 너무 많은 일을 처리하는 경우	작고 목적이 명확한 메소드: 메소드 이름이 직관적이고, 한 가지 일에만 집중하는 것을 말한다
중복 코드: 똑같거나 유사한 코드가 여러 곳에 복사/붙여넣기 된 경우를 말한다	DRY(Don't Repeat Yourself): 중복을 제거하고 공통적인 로직은 메소드/클래스 등으로 추출해 재사용하여 해결한다.
마법의 숫자/문자열: 의미를 알 수 없는 숫자/문자열이 하드코딩 된 상태를 말한다	의미 있는 상수/열거형: 값의 의미를 명확히 설명하는 상수를 사용해 해결한다.
복잡한 조건문: 새로운 조건이 추가될때마다, 조건문이 계속 길어지는 상황을 말한다.	다형성을 활용한 설계: 오버라이딩/오버로딩과 같은 다형성을 활용해, 기능을 추가할 때 기존의 코드를 수정하지 않고 확장할 수 있다
너무 많은 매개변수: 메소드가 너무 많은 파라미터를 필요로하는 경우	객체로 매개변수를 그룹화: 연관된 매개변수들을 별도의 객체로 묶어 전달하여, 그 의도도 명확히 할 수 있다.
'무엇'을 설명하는 주석: 코드를 봐도 알 수 있는 내용을 주석으로 반복하는 것을 말한다. 예) i++; //i를 증가시킴	'왜'를 설명하는 주석: 코드를 작성한 의도/배경/비즈니스 로직 등 코드만으로는 설명하기 어려운 '왜'를 설명한다.

이런 클린코드와 리팩토링이 충돌하는 경우도 생긴다고한다. 둘이 충돌할 경우에는 처한 상황에 맞게 판단하는 것 같았다. 수업 내에서 충돌하는 경우를 몇 개 살펴봤는데 아래와 같다.

상황1: 메소드 분리 vs 과도한 추상화

• 리팩토링: 긴 메소드는 잘게 나누기 "SRP(단일 책임 원칙)"
• 클린코드: 너무 많은 추상화는 코드 흐름을 파악하기 어렵게함

public void processOrder() {
	validateOrder();
    calculateDiscount();
    updateInventory();
    sendConfirmationEmail();
}

클린코드 관점에서 processOrder()에 일이 처리되는 순서대로 각 메소드를 호출하여, 어떤 순서로 프로세스가 이루어지는지를 알 수 있음

다만, 리팩토링의 관점에서는 하나의 메소드 내에서 여러 개의 책임(혹은 동작)이 이루어지고 있기에 아래의 코드와 같이 각 단계를 별도의 클래스로 분리하게됨.

orderValidator.validate();
discountCalculator.calculate();
inventoryManager.update();
emailSender.send();

위에는 각 클래스만다 정말 하나씩의 책임만을 부여하고 있다. 주문은 주문 활성화하는 클래스에서, 할인은 또 해당 클래스에서, 이메일은 이메일 보내는 클래스에서 이렇게 말이다.

이렇게 충돌이 일어나는 경우에는 보통 '변경가능성'과 '협업중심'으로 기준을 세운다고한다.
가독성vs재사용성, 의도 명확성vs구조적 정리 사이의 갈등인 것 같다.

결합도와 응집도

결합도란 우선 말 그대로 무언가가 서로 얼마나 얽혀있나?의 문제이다. 즉, 하나의 클래스가 다른 클래스에 얼마나 의존하고 있는지의 정도에 관한 부분이다. 보통 이 결합도가 낮아서 각 클래스가 서로에게 영향을 적게 미치고 독립적으로 존재해야 좋은 코드라고한다. 후에 나올 "의존성 역전 원칙(DIP)과 "인터페이스 분리 원칙(ISP)"과 관련이 있다.

응집도란 내부가 얼마나 잘 정리되어있는지의 문제이다. 즉, 하나의 클래스가 가진 기능들일 얼마나 서로 연관되어 있는지의 내용이라고 할 수 있다. 후에 나올 "단일 책임 원칙(SRP)"과 관련이 있다.

앞에서 언급된 결합도와 응집도에 대한 정의를 정리하고 가려고한다. 사실 코드를 통해서 안보면 살짝 느낌이 안와서 코드를 보는게 제일 좋지 않나 싶긴하다ㅎ. 아래 코드가 그 예시이다.

class Player {
	Sword sword = new Sword();
    Gun gun = new Gun();
    
    void attack() {
    	sword.slash();
        gun.shoot();
    }
}

이게 결합도가 높은, 좋지 않은 예의 코드이다. Player 클래스 내에서 Sword, Gun 무기를 생성해서 사용하고 있다. 사실 왜 저게 문제인지 와닿지 않을 수 있을 것 같다. 나도 처음에 저렇게 썼기도하고...

정리하면 문제점은 이제 Player가 무기를 바꾼다면 어떻게 할 것이냐에서 발생한다. 예를 들어 총을 버리고 망치라는 무기를 쓴다고 가정하면, 저 클래스 내에 망치라는 무기를 또 생성해줘야한다. 그런데 이제 문제는 코드 실행 중에는 총을 버리는 방법이 없다.

interface Weapon {
	void use();
}

class Sword implements Weapon{ ... }
class Gun implements Weapon { ... }

class Player {
	private Weapon weapon;
    
    public Player (Weapon weapon) {
    	this.weapon = weapon;
    }
    
    public void attack() {
    	weapon.use();
    }
}

위의 코드가 수정 후의 코드이다. 보면 Weapon 이라는 인터페이스를 만들고, Sword와 Gun이 해당 인터페이스를 구현하고 있다. 이로써 Player 클래스 내에서 굳이 무기를 생성할 필요없이 인터페이스인 Weapon을 필드로 두고, 상위 참조하는 방식을 사용할 수 있다. 또, Weapon에는 다양한 하위 무기들이 들어갈 수 있으므로, 생성자를 통해 어떤 무기를 사용할지 외부에서 주입 받을 수 있다. 생성자 뿐만 아니라 메소드를 사용해서도 이런 주입이 가능하다.

class User {
	private String name;
    private String phoneNumber;
    private String email;
    
    public void orderProduct() {
    	...
    }
    
    public void pay() {
    	...
    }
}

위의 예에는 User 클래스 내에 사용자의 이름, 전화번호, 이메일과 같은 개인정보는 물론, 물건의 주문 및 결제를 위한 메소드까지 정의되어있다. 이는 사용자 클래스에 너무 많은 책임을 주고 있다는 점에서 단일 책임 원칙(SRP)을 지키지 못한 것이기도하며, 클래스 내의 기능들의 연관성이 떨어지게된다.

class User {
	private String name;
    private String phoneNumber;
    private String email;
    
    public User (String name, String phoneNumber, String email) {
    	...
    }
    
    public getName () {...}
    ...
    public setEmail () {...}
    ...
}

interface PaymentMethod {
	void pay(int amount);
}

interface OrderProduct {
	void order(String name, int price...);
}

class OrderDetails {	
	private String productName;
    private String price;
    ...
}

일단 대충 수정해보면 위의 코드와 같이 각 기능들과 책임에 따라서 분리하면 될 것 같다는 생각이 들었다.