for-for-if 구조 개선해보기

들어가기 전에

• 이 글은 향로님의 2. 악취나는 코드 개선하기 시리즈 - 심하게 중첩된 코드를 읽고 작성한 글입니다.
• 내용의 출처 또한 향로님의 블로그의 2. 악취나는 코드 개선하기 시리즈 - 심하게 중첩된 코드에서 비롯됨을 밝힙니다.

코드 소개

우리는 학생이라는 객체, 그리고 학생을 묶은 그룹이라는 객체를 가지고 있습니다.

학생

public class Student {
    private String name;

    public Student(String name) {
        this.name = name;
    }

    public String getName() {
        return name;
    }
    // 정적 팩토리 메서드
    public static Student of(String name){
        return new Student(name);
    }
}

그룹

import java.util.*;
public class Group {
    private String groupName;
    private List<Student> member = new ArrayList<>();

    public Group(String groupName) {
        this.groupName = groupName;
    }

    public void addMember(List<Student> students){
        member.addAll(students);
    }
    
    public List<Student> getMembers() {
        return member;
    }    
    
    public String getGroupName() {
        return groupName;
    }    
}

데이터 생성

실제 학생과 그룹 데이터를 다음과 같이 생성했습니다. 우리의 목적은 이름이 "O"인 학생이 어느 그룹에 속하는지 확인하는 것입니다.

import java.util.*;
public class Main {
    public static void main(String[] args) {
        Group groupA = new Group("GROUP-A");
        groupA.addMember(List.of(Student.of("A"),
                Student.of("B"),
                Student.of("C"),
                Student.of("D")));
        Group groupB = new Group("GROUP-B");
        groupB.addMember(List.of(Student.of("E"),
                Student.of("F"),
                Student.of("G"),
                Student.of("H")));
        Group groupC = new Group("GROUP-C");
        groupC.addMember(List.of(Student.of("I"),
                Student.of("J"),
                Student.of("K"),
                Student.of("L")));
        Group groupD = new Group("GROUP-D");
        groupD.addMember(List.of(Student.of("M"),
                Student.of("N"),
                Student.of("O"),
                Student.of("P")));

		List<Group> totalGroup = List.of(groupA, groupB, groupC, groupD);
        // O라는 학생이 어느 그룹에 속하는지 알고 싶습니다.
		String targetName = "O"; 
}

메서드 생성

이제 이름이 targetName인 학생이 어느 그룹에 속하는지 확인하는 메서드를 작성해 보겠습니다.

public static Group findGroupWithTargetMemberIncluded(String targetName, List<Group> totalGroup){
    for (Group group: totalGroup) { // 각각의 그룹에 대해
        for(Student student: group.getMembers()){ // 각 그룹의 학생에 대해 
            if(student.getName().equals(targetName)){ // 만약 학생의 이름이 우리가 찾는 학생의 이름이면
                return group;
            }
        }
    }
    return null;
}

• 위 코드는 상당히 직관적으로 작성됐습니다.
  1. 전체 그룹에서 각각의 그룹을 탐색하고
  2. 각각의 그룹에서는 소속된 학생을 탐색해서
  3. 학생의 이름이 우리가 찾는 학생의 이름과 동일한지를 확인합니다.

좋지 못한 코드와 개선

Refactoring 1

직전에 작성한 findGroupWithTargetMemberIncluded메서드는 Student객체를 직접 노출하고 있습니다. 이는 캡슐화 측면에서 좋지 못하며, 메서드는 역할을 수행하기 위해 Group뿐만 아니라 Student도 알아야 하는 상황입니다. 이를 해결하기 위해 코드를 수정해 보겠습니다.

Group

Group 객체에 다음과 같은 메서드를 추가했습니다.

public List<String> getMemberNames(){
    return member.stream()
            .map(Student::getName)
            .toList();
}

findGroupWithTargetMemberIncluded

새롭게 추가된 메서드를 통해 다음과 같은 코드를 수정할 수 있습니다.

public static Group findGroupWithTargetMemberIncluded(String targetName, List<Group> totalGroup){
    for (Group group: totalGroup) {
        for(String studentName: group.getMemberNames()){
            if(studentName.equals(targetName)){
                return group;
            }
        }
    }
    return null;
}

• 더 이상 Student객체를 findGroupWithTargetMemberIncluded메서드에서 직접적으로 노출시키지 않습니다. findGroupWithTargetMemberIncluded는 Student를 몰라도 정상적으로 동작합니다.

Refactoring 2

for문 안에 for문 안에 if문이 있는 중첩된 구조라는 점도 문제입니다. 지금처럼 단순화된 예제에서는 한눈에 코드를 파악할 수 있지만 만약 복잡한 코드가 for-for-if의 중첩구조를 가진다면 이를 이해하는 건 쉽지 않을 겁니다.

여기서는 '모듈은 자신이 호출한 객체의 내부구조를 몰라야 한다'는 디미터 법칙과 묻지 말고 시켜라(Tell, Don’t ASK)라는 원칙을 적용시킬 수 있습니다.
지금 코드는 getMemberNames메서드를 통해 Group객체의 내부 값인studentName을 직접 가져오고 있습니다. 이는 Group의 내부구조를 findGroupWithTargetMemberIncluded가 알게 됨을 의미합니다.
또한 studentName이 targetName과 동일한지를 묻고있습니다.

Group이 특정 이름의 학생이 본인 그룹에 존재하는지 확인하는 역할을 지니면 외부에 내부구조를 노출시키지 않을 수 있으며, 외부에서는 Group객체에게 targetName의 학생이 존재하는지 알아오도록 지시할 수 있습니다.

group

public boolean hasMember(String targetName){
    for (String studentName: getMemberNames()) {
        if(studentName.equals(targetName)) return true;
    }
    return false;
}

• Group객체는 자신의 멤버 중 targetName이름을 가진 학생이 있는지 확인하는 책임을 가지게 됐습니다.

findGroupWithTargetMemberIncluded

public static Group findGroupWithTargetMemberIncluded(String targetName, List<Group> totalGroup){
    for (Group group: totalGroup) {
        if(group.hasMember(targetName)){
            return group;
        }
    }
    return null;
}

• findGroupWithTargetMemberIncluded는 hasMember메서드를 통해 Group에게 targetName이 존재하는지를 알아오라고 지시하고 있습니다.
• Group이 해결할 수 있는 역할은 Group이 가져감으로써 디미터 법칙을 준수할 수 있었고, findGroupWithTargetMemberIncluded는 중첩구조를 한층 벗겨낸 형태가 되었습니다. (for-for-if구조에서 for-if구조로 변경돼 가독성도 더욱 좋아졌습니다.)

묻는 것과 시키는 것의 차이를 완벽히 이해하기가 어려웠습니다.
저는 묻는 것은 물음의 답을 통해 질문자가 결과를 만들어야 한다면, 시키는 것은 결과 자체를 전달받는 거라고 생각했습니다.
studentName.equals(targetName)는 Group으로부터 studentName이라는 답을 얻은 뒤 findGroupWithTargetMemberIncluded메서드가 직접 equals를 통해 존재여부를 판단하지만,
group.hasMember(targetName)는 Group으로부터 존재여부 자체를 반환받기 때문에 findGroupWithTargetMemberIncluded메서드에서 별도의 판단이 필요하지 않다고 이해했습니다.

Refactoring 3

Group의 hasMember메서드는 Stream을 통해 더욱 간단하고 직관적인 코드로 변경될 수 있습니다.

변경 전

public boolean hasMember(String targetName){
    for (String studentName: getMemberNames()) {
        if(studentName.equals(targetName)) return true;
    }
    return false;
}

변경 후

public boolean hasMember(String targetName){
    return getMemberNames()
            .stream()
            .anyMatch(targetName::equals);
}

코드

• https://github.com/qwerty1434/Refactoring-Of-Nested-Structure

References

• https://jojoldu.tistory.com/177
• https://prohannah.tistory.com/204