[TIL]20241111 스파게티 코드

스파게티 코드는 주로 코드가 구조적이지 않고 뒤얽혀 있어 이해가기 어렵고 유지보수가 복잡해지는 경우를 말한다. 이전에 클린 코드와 코드 리팩토링의 중요성에 대해 다룬 적은 있지만, 실제로 이를 적용해 본 경험은 없었고, 스파게티 코드가 무엇인지도 본 적이 없었다🫨

 

▶스파게티 코드란?

스파게티 코드는 코드의 흐름이 엉망으로 뒤섞여 있어 이해하기 어렵고 유지보수가 힘든 코드를 말한다. 이러한 코드는 반복된 로직, 무분별한 의존성, 가독성이 떨어지는 구조 등으로 인해 복잡해진다. 일반적으로 스파게티 코드는 특정 기능을 추가하거나 변경할 때 예상치 못한 버그를 발생시키기 쉽고, 개발자 간의 협업을 어렵게 만든다.

---

▶ 코드 분석

내가 작성한 GamingSetupService 클래스의 코드로, 이 코드는 기본적으로 잘 동작하지만, 구조적으로 개선할 여지가 많다.

@Service
public class GamingSetupService {

    private final GamingSetupRepository gamingSetupRepository;

    public GamingSetupService(GamingSetupRepository gamingSetupRepository) {
        this.gamingSetupRepository = gamingSetupRepository;
    }

    public List<GamingSetupResponseDto> getAllGamingSetups() {
        return gamingSetupRepository.findAll().stream()
                .map(this::convertToResponseDto)
                .collect(Collectors.toList());
    }

    public Optional<GamingSetupResponseDto> getGamingSetupByPlayer(String player) {
        return gamingSetupRepository.findByPlayer(player).map(this::convertToResponseDto);
    }

    public GamingSetupResponseDto createGamingSetup(GamingSetupRequestDto gamingSetupRequestDto) {
        GamingSetup gamingSetup = convertToEntity(gamingSetupRequestDto);
        GamingSetup savedSetup = gamingSetupRepository.save(gamingSetup);
        return convertToResponseDto(savedSetup);
    }

    public Optional<GamingSetupResponseDto> updateGamingSetup(String player, GamingSetupRequestDto gamingSetupRequestDto) {
        Optional<GamingSetup> existingSetup = gamingSetupRepository.findByPlayer(player);
        if (existingSetup.isPresent()) {
            GamingSetup gamingSetup = existingSetup.get();
            updateEntityFromDto(gamingSetupRequestDto, gamingSetup);
            GamingSetup updatedSetup = gamingSetupRepository.save(gamingSetup);
            return Optional.of(convertToResponseDto(updatedSetup));
        }
        return Optional.empty();
    }

    public Optional<GamingSetupResponseDto> partialUpdateGamingSetup(String player, GamingSetupRequestDto gamingSetupRequestDto) {
        Optional<GamingSetup> existingSetup = gamingSetupRepository.findByPlayer(player);
        if (existingSetup.isPresent()) {
            GamingSetup gamingSetup = existingSetup.get();
            updateEntityFromDto(gamingSetupRequestDto, gamingSetup);
            GamingSetup updatedSetup = gamingSetupRepository.save(gamingSetup);
            return Optional.of(convertToResponseDto(updatedSetup));
        }
        return Optional.empty();
    }

    public boolean deleteGamingSetup(String player) {
        Optional<GamingSetup> existingSetup = gamingSetupRepository.findByPlayer(player);
        if (existingSetup.isPresent()) {
            gamingSetupRepository.delete(existingSetup.get());
            return true;
        }
        return false;
    }

    private GamingSetup convertToEntity(GamingSetupRequestDto dto) {
        return new GamingSetup(
                dto.getPlatform(),
                dto.getPlayer(),
                dto.getDpi(),
                dto.getGeneralSens(),
                dto.getVerticalSens(),
                dto.getAimSens(),
                dto.getAimSens1x(),
                dto.getAimSens2x(),
                dto.getAimSens3x(),
                dto.getAimSens4x(),
                dto.getAimSens6x(),
                dto.getAimSens8x(),
                dto.getAimSens15x(),
                dto.getResolution(),
                dto.getFov(),
                dto.getMonitor(),
                dto.getMouse(),
                dto.getMousepad(),
                dto.getKeyboard(),
                dto.getHeadset(),
                dto.getSoundCard()
        );
    }

    private GamingSetupResponseDto convertToResponseDto(GamingSetup entity) {
        return new GamingSetupResponseDto(
                entity.getId(),
                entity.getPlatform(),
                entity.getPlayer(),
                entity.getDpi(),
                entity.getGeneralSens(),
                entity.getVerticalSens(),
                entity.getAimSens(),
                entity.getAimSens1x(),
                entity.getAimSens2x(),
                entity.getAimSens3x(),
                entity.getAimSens4x(),
                entity.getAimSens6x(),
                entity.getAimSens8x(),
                entity.getAimSens15x(),
                entity.getResolution(),
                entity.getFov(),
                entity.getMonitor(),
                entity.getMouse(),
                entity.getMousepad(),
                entity.getKeyboard(),
                entity.getHeadset(),
                entity.getSoundCard()
        );
    }

    private void updateEntityFromDto(GamingSetupRequestDto dto, GamingSetup entity) {
        if (dto.getPlatform() != null && !dto.getPlatform().isEmpty()) entity.setPlatform(dto.getPlatform());
        if (dto.getPlayer() != null && !dto.getPlayer().isEmpty()) entity.setPlayer(dto.getPlayer());
        if (dto.getDpi() != 0) entity.setDpi(dto.getDpi());
        if (dto.getGeneralSens() != 0) entity.setGeneralSens(dto.getGeneralSens());
        if (dto.getVerticalSens() != 0) entity.setVerticalSens(dto.getVerticalSens());
        if (dto.getAimSens() != 0) entity.setAimSens(dto.getAimSens());
        if (dto.getAimSens1x() != 0) entity.setAimSens1x(dto.getAimSens1x());
        if (dto.getAimSens2x() != 0) entity.setAimSens2x(dto.getAimSens2x());
        if (dto.getAimSens3x() != 0) entity.setAimSens3x(dto.getAimSens3x());
        if (dto.getAimSens4x() != 0) entity.setAimSens4x(dto.getAimSens4x());
        if (dto.getAimSens6x() != 0) entity.setAimSens6x(dto.getAimSens6x());
        if (dto.getAimSens8x() != 0) entity.setAimSens8x(dto.getAimSens8x());
        if (dto.getAimSens15x() != 0) entity.setAimSens15x(dto.getAimSens15x());
        if (dto.getResolution() != null && !dto.getResolution().isEmpty()) entity.setResolution(dto.getResolution());
        if (dto.getFov() != 0) entity.setFov(dto.getFov());
        if (dto.getMonitor() != null && !dto.getMonitor().isEmpty()) entity.setMonitor(dto.getMonitor());
        if (dto.getMouse() != null && !dto.getMouse().isEmpty()) entity.setMouse(dto.getMouse());
        if (dto.getMousepad() != null && !dto.getMousepad().isEmpty()) entity.setMousepad(dto.getMousepad());
        if (dto.getKeyboard() != null && !dto.getKeyboard().isEmpty()) entity.setKeyboard(dto.getKeyboard());
        if (dto.getHeadset() != null && !dto.getHeadset().isEmpty()) entity.setHeadset(dto.getHeadset());
        if (dto.getSoundCard() != null && !dto.getSoundCard().isEmpty()) entity.setSoundCard(dto.getSoundCard());
    }
}

 

▶스파게티 코드의 문제점

이 코드는 기능적으로 잘 동작하지만, 유지보수성이 떨어지는 스파게티 코드로 분류될 수 있다. 

중복된 로직

updateGamingSetup과 partialUpdateGamingSetup 메서드는 거의 동일한 로직을 반복하고 있다. 중복된 로직은 유지보수를 어렵게 만들고, 코드 변경 시 오류를 발생시킬 확률을 높인다.

긴 메서드

convertToEntity, convertToResponseDto, 그리고 updateEntityFromDto 메서드는 모두 길고 많은 필드를 다루고 있다. 이는 가독성을 저하시켜 코드 파악을 어렵게 만든다.

명확하지 않은 역할

서비스 클래스는 비즈니스 로직을 처리해야 하지만, 현재 코드에서는 DTO와 엔티티 간의 변환 로직이 서비스 클래스에 포함되어 있다. 이는 서비스 클래스의 역할을 모호하게 만들고 코드 복잡도를 높인다.

---

▷ 예시로 만들어본 스파게티 코드

- 의도적으로 여러 기능을 섞고, 메서드가 길고, 코드가 중복되고, 모듈화가 부족한 형태로 구현.

@Service
public class GamingSetupService {

    private final GamingSetupRepository gamingSetupRepository;

    public GamingSetupService(GamingSetupRepository gamingSetupRepository) {
        this.gamingSetupRepository = gamingSetupRepository;
    }

    public Object handleGamingSetup(String action, String player, GamingSetupRequestDto dto) {
        if ("getAll".equals(action)) {
            return gamingSetupRepository.findAll().stream()
                    .map(entity -> new GamingSetupResponseDto(
                            entity.getId(), entity.getPlatform(), entity.getPlayer(), entity.getDpi(),
                            entity.getGeneralSens(), entity.getVerticalSens(), entity.getAimSens(),
                            entity.getAimSens1x(), entity.getAimSens2x(), entity.getAimSens3x(), entity.getAimSens4x(),
                            entity.getAimSens6x(), entity.getAimSens8x(), entity.getAimSens15x(), entity.getResolution(),
                            entity.getFov(), entity.getMonitor(), entity.getMouse(), entity.getMousepad(),
                            entity.getKeyboard(), entity.getHeadset(), entity.getSoundCard()))
                    .collect(Collectors.toList());
        } else if ("getByPlayer".equals(action)) {
            Optional<GamingSetup> result = gamingSetupRepository.findByPlayer(player);
            if (result.isPresent()) {
                GamingSetup entity = result.get();
                return new GamingSetupResponseDto(
                        entity.getId(), entity.getPlatform(), entity.getPlayer(), entity.getDpi(),
                        entity.getGeneralSens(), entity.getVerticalSens(), entity.getAimSens(),
                        entity.getAimSens1x(), entity.getAimSens2x(), entity.getAimSens3x(), entity.getAimSens4x(),
                        entity.getAimSens6x(), entity.getAimSens8x(), entity.getAimSens15x(), entity.getResolution(),
                        entity.getFov(), entity.getMonitor(), entity.getMouse(), entity.getMousepad(),
                        entity.getKeyboard(), entity.getHeadset(), entity.getSoundCard());
            } else {
                return Optional.empty();
            }
        } else if ("create".equals(action)) {
            GamingSetup entity = new GamingSetup(
                    dto.getPlatform(), dto.getPlayer(), dto.getDpi(), dto.getGeneralSens(), dto.getVerticalSens(),
                    dto.getAimSens(), dto.getAimSens1x(), dto.getAimSens2x(), dto.getAimSens3x(), dto.getAimSens4x(),
                    dto.getAimSens6x(), dto.getAimSens8x(), dto.getAimSens15x(), dto.getResolution(), dto.getFov(),
                    dto.getMonitor(), dto.getMouse(), dto.getMousepad(), dto.getKeyboard(), dto.getHeadset(),
                    dto.getSoundCard());
            return new GamingSetupResponseDto(
                    gamingSetupRepository.save(entity).getId(), entity.getPlatform(), entity.getPlayer(), entity.getDpi(),
                    entity.getGeneralSens(), entity.getVerticalSens(), entity.getAimSens(), entity.getAimSens1x(),
                    entity.getAimSens2x(), entity.getAimSens3x(), entity.getAimSens4x(), entity.getAimSens6x(),
                    entity.getAimSens8x(), entity.getAimSens15x(), entity.getResolution(), entity.getFov(),
                    entity.getMonitor(), entity.getMouse(), entity.getMousepad(), entity.getKeyboard(),
                    entity.getHeadset(), entity.getSoundCard());
        } else if ("update".equals(action) || "partialUpdate".equals(action)) {
            Optional<GamingSetup> existingSetup = gamingSetupRepository.findByPlayer(player);
            if (existingSetup.isPresent()) {
                GamingSetup gamingSetup = existingSetup.get();
                if (dto.getPlatform() != null && !dto.getPlatform().isEmpty()) gamingSetup.setPlatform(dto.getPlatform());
                if (dto.getPlayer() != null && !dto.getPlayer().isEmpty()) gamingSetup.setPlayer(dto.getPlayer());
                if (dto.getDpi() != 0) gamingSetup.setDpi(dto.getDpi());
                if (dto.getGeneralSens() != 0) gamingSetup.setGeneralSens(dto.getGeneralSens());
                if (dto.getVerticalSens() != 0) gamingSetup.setVerticalSens(dto.getVerticalSens());
                if (dto.getAimSens() != 0) gamingSetup.setAimSens(dto.getAimSens());
                if (dto.getAimSens1x() != 0) gamingSetup.setAimSens1x(dto.getAimSens1x());
                if (dto.getAimSens2x() != 0) gamingSetup.setAimSens2x(dto.getAimSens2x());
                if (dto.getAimSens3x() != 0) gamingSetup.setAimSens3x(dto.getAimSens3x());
                if (dto.getAimSens4x() != 0) gamingSetup.setAimSens4x(dto.getAimSens4x());
                if (dto.getAimSens6x() != 0) gamingSetup.setAimSens6x(dto.getAimSens6x());
                if (dto.getAimSens8x() != 0) gamingSetup.setAimSens8x(dto.getAimSens8x());
                if (dto.getAimSens15x() != 0) gamingSetup.setAimSens15x(dto.getAimSens15x());
                if (dto.getResolution() != null && !dto.getResolution().isEmpty()) gamingSetup.setResolution(dto.getResolution());
                if (dto.getFov() != 0) gamingSetup.setFov(dto.getFov());
                if (dto.getMonitor() != null && !dto.getMonitor().isEmpty()) gamingSetup.setMonitor(dto.getMonitor());
                if (dto.getMouse() != null && !dto.getMouse().isEmpty()) gamingSetup.setMouse(dto.getMouse());
                if (dto.getMousepad() != null && !dto.getMousepad().isEmpty()) gamingSetup.setMousepad(dto.getMousepad());
                if (dto.getKeyboard() != null && !dto.getKeyboard().isEmpty()) gamingSetup.setKeyboard(dto.getKeyboard());
                if (dto.getHeadset() != null && !dto.getHeadset().isEmpty()) gamingSetup.setHeadset(dto.getHeadset());
                if (dto.getSoundCard() != null && !dto.getSoundCard().isEmpty()) gamingSetup.setSoundCard(dto.getSoundCard());

                GamingSetup updatedSetup = gamingSetupRepository.save(gamingSetup);
                return new GamingSetupResponseDto(
                        updatedSetup.getId(), updatedSetup.getPlatform(), updatedSetup.getPlayer(), updatedSetup.getDpi(),
                        updatedSetup.getGeneralSens(), updatedSetup.getVerticalSens(), updatedSetup.getAimSens(),
                        updatedSetup.getAimSens1x(), updatedSetup.getAimSens2x(), updatedSetup.getAimSens3x(),
                        updatedSetup.getAimSens4x(), updatedSetup.getAimSens6x(), updatedSetup.getAimSens8x(),
                        updatedSetup.getAimSens15x(), updatedSetup.getResolution(), updatedSetup.getFov(),
                        updatedSetup.getMonitor(), updatedSetup.getMouse(), updatedSetup.getMousepad(),
                        updatedSetup.getKeyboard(), updatedSetup.getHeadset(), updatedSetup.getSoundCard());
            }
            return Optional.empty();
        } else if ("delete".equals(action)) {
            Optional<GamingSetup> existingSetup = gamingSetupRepository.findByPlayer(player);
            if (existingSetup.isPresent()) {
                gamingSetupRepository.delete(existingSetup.get());
                return true;
            }
            return false;
        }
        return null;
    }
}

▷ 문제점 분석

→ 모든 기능이 하나의 메서드에서 처리되며, 이로 인해 이해하기 어렵고 유지보수하기 힘들어진다.

→ 긴 조건문이 많아 가독성이 떨어진다.

→ 단일 책임 원칙 위반: 메서드가 여러 역할을 수행하여 코드의 모듈화가 어렵다.

→ 복잡성 증가: 각 작업이 서로 얽혀 있어 코드 수정 시 다른 부분에 영향을 미칠 가능성이 높다.

---

▶개선 방안

• 중복 제거: updateGamingSetup과 partialUpdateGamingSetup의 공통된 부분을 별도의 메서드로 분리하여 중복을 제거한다. 이를 통해 코드의 재사용성을 높이고 유지보수를 쉽게 할 수 있다.

▷중복 제거 예시

private Optional<GamingSetupResponseDto> updateOrPartialUpdateGamingSetup(String player, GamingSetupRequestDto gamingSetupRequestDto) {
    Optional<GamingSetup> existingSetup = gamingSetupRepository.findByPlayer(player);
    if (existingSetup.isPresent()) {
        GamingSetup gamingSetup = existingSetup.get();
        updateEntityFromDto(gamingSetupRequestDto, gamingSetup);
        GamingSetup updatedSetup = gamingSetupRepository.save(gamingSetup);
        return Optional.of(convertToResponseDto(updatedSetup));
    }
    return Optional.empty();
}

→ 위의 메서드를 사용하여 updateGamingSetup과 partialUpdateGamingSetup의 중복된 로직을 제거.

---

• 메서드 분리: 긴 메서드를 작은 단위로 분리하여 가독성을 높인다. 예를 들어, updateEntityFromDto 메서드를 필드별로 업데이트하는 메서드로 분리할 수 있다.

▷메서드 분리 예시

private void updateEntityFromDto(GamingSetupRequestDto dto, GamingSetup entity) {
    if (dto.getPlatform() != null && !dto.getPlatform().isEmpty()) updatePlatform(dto, entity);
    if (dto.getPlayer() != null && !dto.getPlayer().isEmpty()) updatePlayer(dto, entity);
    // 나머지 필드도 동일하게 분리
}

private void updatePlatform(GamingSetupRequestDto dto, GamingSetup entity) {
    entity.setPlatform(dto.getPlatform());
}

private void updatePlayer(GamingSetupRequestDto dto, GamingSetup entity) {
    entity.setPlayer(dto.getPlayer());
}

→ 필드별로 업데이트하는 메서드를 분리하여 updateEntityFromDto의 가독성↑

---

• 매퍼 클래스 도입: DTO와 엔티티 간의 변환 로직을 별도의 매퍼 클래스로 분리한다. 이를 통해 서비스 클래스는 비즈니스 로직에만 집중할 수 있고, 코드의 책임을 명확하게 분리할 수 있다.

▷매퍼 클래스 도입 예시

public class GamingSetupMapper {
    public static GamingSetup convertToEntity(GamingSetupRequestDto dto) {
        return new GamingSetup(
                dto.getPlatform(),
                dto.getPlayer(),
                dto.getDpi(),
                dto.getGeneralSens(),
                dto.getVerticalSens(),
                dto.getAimSens(),
                dto.getAimSens1x(),
                dto.getAimSens2x(),
                dto.getAimSens3x(),
                dto.getAimSens4x(),
                dto.getAimSens6x(),
                dto.getAimSens8x(),
                dto.getAimSens15x(),
                dto.getResolution(),
                dto.getFov(),
                dto.getMonitor(),
                dto.getMouse(),
                dto.getMousepad(),
                dto.getKeyboard(),
                dto.getHeadset(),
                dto.getSoundCard()
        );
    }

    public static GamingSetupResponseDto convertToResponseDto(GamingSetup entity) {
        return new GamingSetupResponseDto(
                entity.getId(),
                entity.getPlatform(),
                entity.getPlayer(),
                entity.getDpi(),
                entity.getGeneralSens(),
                entity.getVerticalSens(),
                entity.getAimSens(),
                entity.getAimSens1x(),
                entity.getAimSens2x(),
                entity.getAimSens3x(),
                entity.getAimSens4x(),
                entity.getAimSens6x(),
                entity.getAimSens8x(),
                entity.getAimSens15x(),
                entity.getResolution(),
                entity.getFov(),
                entity.getMonitor(),
                entity.getMouse(),
                entity.getMousepad(),
                entity.getKeyboard(),
                entity.getHeadset(),
                entity.getSoundCard()
        );
    }
}

→ 매퍼 클래스를 도입하여 DTO와 엔티티 간의 변환 로직을 서비스 클래스에서 분리.

---

▶클린 코드 적용 결과

위의 개선 방안을 적용하면 코드의 가독성과 유지보수성이 크게 향상될 것이다. 중복된 로직이 제거되고, 각 클래스와 메서드의 역할이 명확해져 코드의 품질이 개선된다. 또한, 새로운 기능을 추가하거나 기존 기능을 수정할 때 발생할 수 있는 오류를 줄일 수 있다.