본문 바로가기
휴게소

코딩 실력이 늘었다고 생각할 때(feat. 리팩토링)

by Vintz 2021. 8. 28.
반응형

기능은 그대로 유지 보수는 ⬆️

요즘은 예전에 만들었던 프로젝트 리팩토링을 하는 것에 시간을 많이 쏟고 있다. 어쩌다 구경을 하게 되었는데 고칠 점이 너무 많아 보여서 리팩토링을 하기로 결심했다. 하다보니 재밌기도 해서 시간이 잘간다.

 

먼저 간략하게 표현한 예전 코드는 다음과 같다.

// 📂 main.js

// XHR 설정
const getJSON = function(url, callback) {
	// ...XHR
}

// API 호출
getJSON(
    'http://api.openweathermap...', 
    function (err, data) {
    	if (err면) { alert() }
        else {
        	// 성공이면 데이터 표시
        	loadWeather(data);
      		todayClothes(data);
      		loadImg(data);
        }
    }
)

// 날씨 관련 함수
function loadWeather(data) {
 	// ...날씨 관련 로직
}

// 옷차림 추천 함수
function todayClothes(data) {
	// ...옷차림 추천 로직
}

// 배경 변경 함수
function loadImg(data) {
	// ...배경 변경 로직
}

잠시 코드를 보고, 현재 내 수준에서 당장 리팩토링을 해야할 부분 몇 가지가 보였다.

  1. 모든 로직이 main.js에 있다.
  2. 함수 내 코드가 너무 길고, 유지보수 하기가 어렵게 되어있다.

프로젝트의 볼륨이 크진 않지만 코드량이 많고, 직관적이지 않았다. 여기서 내가 생각하는 리팩토링이란 '기능은 유지하되 가독성과 유지보수가 좋게' 만드는 것이기 때문에 이 원칙을 따라서 코드를 짜게 되었다.

  1. 모듈화 진행(API 호출 및 함수별 모듈화)
  2. 함수 내 기능은 그대로. 가독성 ⬆️ 유지보수 ⬆️

API 호출 로직은 service 폴더에 따로 class로 만들어 빼주었고, 함수들도 모듈화를 해주었다.

├── 📂 src 
│   ├── 📂 service
│   │	├── weather.js
│   ├── background.js
│   ├── clothes.js
│   ├── config.js
│   ├── main.js
│   └── temperature.js

함수 내 코드는 대표로 background.js를 가져와봤다.

// background.js before 📂

function loadImg(data) {
  let background = document.querySelector('.flex-container');
  let currentTemp = data.main.temp;

  let winter = currentTemp <= 4;
  let earlyWinter = currentTemp >= 5 && currentTemp < 9;
  let beginWinter = currentTemp >= 9 && currentTemp < 12;
  let fall = currentTemp >= 12 && currentTemp < 17;
  let earlyFall = currentTemp >= 17 && currentTemp < 20;
  let earlySummer = currentTemp >= 20 && currentTemp < 23;
  let beginSummer = currentTemp >= 23 && currentTemp < 28;
  let summer = currentTemp >= 28;

  if (winter) {
    background.style.backgroundImage = "url('./img/winter.jpg')";
  } else if (earlyWinter) {
    background.style.backgroundImage = "url('./img/earlywinter.jpg')";
  } else if (beginWinter) {
    background.style.backgroundImage = "url('./img/beginwinter.jpg')";
  } else if (fall) {
    background.style.backgroundImage = "url('./img/fall.jpg')";
  } else if (earlyFall) {
    background.style.backgroundImage = "url('./img/earlyfall.jpg')";
  } else if (earlySummer) {
    background.style.backgroundImage = "url('./img/earlysummer.jpg')";
  } else if (beginSummer) {
    background.style.backgroundImage = "url('./img/beginsummer.jpg')";
  } else if (summer) {
    background.style.backgroundImage = "url('./img/summer.jpg')";
  }
}

온도에 따라 배경화면이 바뀌는 로직을 구현했다. 그때 당시에는 가독성을 좋게 하기 위해 온도 범위마다 변수를 선언했던 것 같다. 그리고 몇몇 중복되는 코드와 너무 많은 if문을 고치고 싶었다.

// background.js after 📂

export default function background(data) {
  const background = document.querySelector('.flex-container');
  const temp = data.main.temp;
  const season = (url) => {
    background.style.backgroundImage = `url(${url})`;
  };

  if (temp <= 4) {
    return season('./img/winter.jpg');
  }
  if (temp >= 5 && temp < 9) {
    return season('./img/earlywinter.jpg');
  }
  if (temp >= 9 && temp < 12) {
    return season('./img/beginwinter.jpg');
  }
  if (temp >= 12 && temp < 17) {
    return season('./img/fall.jpg');
  }
  if (temp >= 17 && temp < 20) {
    return season('./img/earlyfall.jpg');
  }
  if (temp >= 20 && temp < 23) {
    return season('./img/earlysummer.jpg');
  }
  if (temp >= 23 && temp < 28) {
    return season('./img/beginsummer.jpg');
  }
  if (temp >= 28) {
    return season('./img/summer.jpg');
  }
  
  console.error(new Error(`${temp}, 배경을 불러오지 못함.`));
}

처음에는 switch문으로 시도해봤는데 case에는 비교연산자를 사용할 수 없다는 것을 이번에 알게 되었다..어쨌든 최대한 가독성이 좋고 유지보수를 좋게하기 위해 노력했다.

  1. 굳이 많은 수의 온도범위를 변수로 만들 필요가 없었다.(오히려 더 헷갈렸다.)
  2. else if..else if..이런식의 중첩보단 어차피 return후엔 함수가 종료되니 여러개의 if문으로 가독성을 좋게 한다.
  3. 중복되는 코드(url 부분)를 함수로 만들었다. 가독성 ⬆️ 유지보수성 ⬆️

깔끔해진 main.js

// main.js 📂

'use strict';
import apiKey from './config.js';
import Weather from './service/weather.js';
import temperature from './temperature.js';
import clothes from './clothes.js';
import background from './background.js';

const weather = new Weather(apiKey);

weather.getJSON((err, data) => {
  if (err !== null) {
    console.error(new Error(`${err}, 에러 발생.`));
  } else {
    temperature(data);
    background(data);
    clothes(data);
    date(data);
  }
});

function date() {
  let time = document.querySelector('.time');
  let date = new Date();
  let month = date.getMonth() + 1;
  let day = date.getDate();
  let hours = date.getHours();
  let minutes = date.getMinutes();

  time.append(`${month}월 ${day}일 ${hours}시 ${minutes}분`);
}

 background.js의 if문을 더 줄이고 싶지만 현재는 이정도로 만족한다. 애초에 설계를 잘못했나 싶기도..💩

 

반응형