[iOS swift] 리팩토링

언제 한번 리팩토링을 수행하고 그 전후 차이를 비교하는 글을 남기고 싶었는데, 때가 온 것 같다.

수 개월에 걸친 개인 프로젝트를 완료하고 심사를 위해 앱을 제출한 상태에서 더러워진 코드를 정리하기 위해 리팩토링을 수행하려 한다. 그리고 그 전후 차이를 기록으로 남긴다. 

 

 

리팩토링 전

일단 매우 보기 싫은 리팩토링 전의 코드는 아래와 같다: (255줄)

import UIKit
import Firebase
import CodableFirebase
import CoreLocation


class ListStoreViewController: UIViewController {

    @IBOutlet weak var storeTableView: UITableView!
    @IBOutlet weak var categoryCollectionView: UICollectionView!
    @IBOutlet weak var titleLabel: UILabel!
    
    var ref: DatabaseReference!
    let storage = Storage.storage()
    let categories: [Category] =
        [Category.all,
         Category.koreanAsian,
         Category.japaneseCutlet,
         Category.chinese,
         Category.western,
         Category.chickenPizza,
         Category.bunsik,
         Category.mexican,
         Category.fastFood,
         Category.meat,
         Category.cafe]
    
    
    var stores = [Store]()
    var filteredStores = [Store]()
    var countForJustExecuteOnce: Bool = false
    
    let refreshControl = UIRefreshControl()

    override func viewDidLoad() {
        super.viewDidLoad()
        
        LoadingService.showLoading()
        
        self.storeTableView.delegate = self
        self.storeTableView.dataSource = self
        self.categoryCollectionView.delegate = self
        self.categoryCollectionView.dataSource = self
        
        NotificationCenter.default.addObserver(self,
                                               selector: #selector(self.didReceiveCategorySelectedNotification(_:)),
                                               name: Notification.Name("categorySelected"), object: nil)
        NotificationCenter.default.addObserver(self,
                                               selector: #selector(self.didReceiveFilterSelectedNotification(_:)),
                                               name: Notification.Name("filterSelected"), object: nil)
        ref = Database.database(url: "https://restaurants-e62b0-default-rtdb.asia-southeast1.firebasedatabase.app").reference()

        self.refreshControl.attributedTitle = NSAttributedString(string: "")
        self.refreshControl.addTarget(self, action: #selector(onPullToRefresh), for: .valueChanged)
        self.storeTableView.addSubview(refreshControl)
        
        getStoresInfoFromDatabase()
        setCollectionViewLayout()
    }
    
    
    override func viewWillAppear(_ animated: Bool) {
        super.viewWillAppear(animated)
        
        setUI() // view will appear에 두는 이유는 다음 화면에서 다시 돌아올 때를 대비하기 위해서.
        NotificationCenter.default.post(name: Notification.Name("changeToCurrentFilter"),
                                        object: nil)
    }
    

    @objc func didReceiveCategorySelectedNotification(_ noti: Notification) {
        guard let category = noti.userInfo?["category"] as? String,
              let currentCategory = self.titleLabel?.text
        else { print("can't handle didReceiveCategory Notification"); return; }
        
        if category != currentCategory { executeCategoryFiltering(category) }
        
        DispatchQueue.main.async { self.titleLabel.text = category }
    }
    
    private func executeCategoryFiltering(_ category: String) {
        switch category {
        case Category.all.rawValue:
            self.filteredStores = self.stores.sorted(by: { $0.storeInfo.createDate < $1.storeInfo.createDate })
        case Category.koreanAsian.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.koreanAsian.rawValue })
        case Category.japaneseCutlet.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.japaneseCutlet.rawValue })
        case Category.chinese.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.chinese.rawValue })
        case Category.western.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.western.rawValue })
        case Category.chickenPizza.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.chickenPizza.rawValue })
        case Category.bunsik.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.bunsik.rawValue })
        case Category.mexican.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.mexican.rawValue })
        case Category.fastFood.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.fastFood.rawValue })
        case Category.meat.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.meat.rawValue })
        case Category.cafe.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.cafe.rawValue })
        default:
            self.filteredStores = self.stores.sorted(by: { $0.storeInfo.createDate < $1.storeInfo.createDate })
            print("in default category filter")
        }
        
        DispatchQueue.main.async {
            NotificationCenter.default.post(name: Notification.Name("categoryDidChange"),
                                            object: nil,
                                            userInfo: nil)
            self.storeTableView.reloadData()
        }
    }
    
    @objc func didReceiveFilterSelectedNotification(_ noti: Notification) {
        guard let filter = noti.userInfo?["filter"] as? String
        else { print("can't handle didReceiveFilter noti"); return }
        
        switch filter {
        case Filter.latest.filterName:
            self.filteredStores = self.filteredStores.sorted(by: { $0.storeInfo.createDate > $1.storeInfo.createDate })
        case Filter.nearest.filterName:
            self.filteredStores = self.filteredStores.sorted(
                by: { self.getDistanceFromCurrentLocation($0.storeInfo.latitude, $0.storeInfo.longtitude) <
                    self.getDistanceFromCurrentLocation($1.storeInfo.latitude, $1.storeInfo.longtitude) })
        case Filter.ratingHigh.filterName:
            self.filteredStores = self.filteredStores.sorted(by: { $0.storeInfo.rating > $1.storeInfo.rating })
        case Filter.reviewCount.filterName:
            self.filteredStores = self.filteredStores.sorted(by: { $0.storeInfo.commentCount > $1.storeInfo.commentCount })
        case Filter.oldest.filterName:
            self.filteredStores = self.filteredStores.sorted(by: { $0.storeInfo.createDate < $1.storeInfo.createDate })
        case Filter.ratingLow.filterName:
            self.filteredStores = self.filteredStores.sorted(by: { $0.storeInfo.rating < $1.storeInfo.rating })
        default:
            self.filteredStores = self.filteredStores.sorted(by: { $0.storeInfo.createDate > $1.storeInfo.createDate })
            print("default filter notification in")
        }
        
        DispatchQueue.main.async { self.storeTableView.reloadData() }
    }
    
    private func getDistanceFromCurrentLocation(_ targetLatitude: Double, _ targetLongitude: Double) -> CLLocationDistance {
        let currentLocationLatitude = UserDefaults.standard.double(forKey: "currentLocationLatitude")
        let currentLocationLongitude = UserDefaults.standard.double(forKey: "currentLocationLongitude")
        
        let targetLocation = CLLocationCoordinate2D(latitude: targetLatitude, longitude: targetLongitude)
        let currentLocation = CLLocationCoordinate2D(latitude: currentLocationLatitude, longitude: currentLocationLongitude)
        
        return targetLocation.distance(from: currentLocation)
    }
    
    private func executeFilterCategoryFiltering(_ filter: String) {
        
    }

    private func setUI() {
        self.setStatusBarBackgroundColor()
        self.setNavigationBarBackgroundColor()
        self.storeTableView.backgroundColor = Config.shared.application60Color
        self.setNavigationBarHidden(isHidden: true)
        
    }

    private func setCollectionViewLayout() {
        if let layout = categoryCollectionView.collectionViewLayout as? UICollectionViewFlowLayout {
            //this value represents the minimum spacing between items in the same column.
            layout.minimumInteritemSpacing = 30
            //this value represents the minimum spacing between successive columns.
            layout.minimumLineSpacing = 30
            layout.sectionInset = UIEdgeInsets(top: 0, left: 20, bottom: 0, right: 20)
        }
    }
    
    @objc func onPullToRefresh() {
        if self.filteredStores.isEmpty == true {
            getStoresInfoFromDatabase()
        }
        DispatchQueue.main.async {
            self.refreshControl.endRefreshing()
        }
        
        
    }
    
    private func getStoresInfoFromDatabase() {
        self.ref.child("stores").observe(DataEventType.value, with: { (snapshot) in
            
            if snapshot.exists() {
                self.stores = []
                guard let value = snapshot.value else {return}
                do {
                    let storesData = try FirebaseDecoder().decode([String: StoreInfo].self, from: value)
                    
                    for storeData in storesData {
                        let store: Store = Store(storeKey: storeData.key, storeInfo: storeData.value)
                        self.stores.append(store)
                    }
                    self.filteredStores = self.stores.sorted(by: { $0.storeInfo.createDate < $1.storeInfo.createDate })
                    
                    DispatchQueue.main.async {
                        NotificationCenter.default.post(name: Notification.Name("changeToCurrentFilter"),
                                                        object: nil)
                    }
                } catch let err {
                    print(err.localizedDescription)
                }
            }
        })
    }
}

extension ListStoreViewController: UITableViewDelegate {
    func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
        return self.filteredStores.count
    }
    
    func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        guard let cell = self.storeTableView.dequeueReusableCell(withIdentifier: StoreTableViewCell.reuseIdentifier) as? StoreTableViewCell
        else { return UITableViewCell() }
        
        let store = self.filteredStores[indexPath.row]
        
        cell.nameLabel?.text = store.storeInfo.name
        cell.addressLabel?.text = store.storeInfo.address
        cell.rateLabel?.text = "\(store.storeInfo.rating)(\(store.storeInfo.commentCount))"
        cell.categoryLabel?.text = store.storeInfo.category
        
        let storageRef = storage.reference()
        let reference = storageRef.child("\(store.storeInfo.mainImage)")
        let placeholderImage = UIImage(named: "placeholder.jpg")
        cell.storeImageView.sd_setImage(with: reference, placeholderImage: placeholderImage)
        
        hideLoadingWhenTableViewDidAppear(indexPath)
        return cell
    }
    
    private func hideLoadingWhenTableViewDidAppear(_ indexPath: IndexPath) {
        if indexPath.row == self.filteredStores.count - 1 { LoadingService.hideLoading() }
    }
}

extension ListStoreViewController: UITableViewDataSource {
    func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
        StoreSingleton.shared.store = self.filteredStores[indexPath.row]
        
        guard let viewController = self.storyboard?.instantiateViewController(withIdentifier: "DetailViewController") as? DetailViewController
        else { return }
            
        tableView.deselectRow(at: indexPath, animated: true)
        self.navigationController?.pushViewController(viewController, animated: true)
    }
}

 

 

다들 공감하겠지만 처음부터 끝까지 읽기도 싫다. 그런데 이게 리팩토링을 한다고 읽기 좋을 지.. 한번 비교해보고 각자 판단해보자.

 

 

리팩토링 후

먼저 코드 줄 수는 얼마나 바뀌었을 것 같은가? 

10% 이상 줄었다, 20% 이상 줄었다 라는 생각을 했겠지만 그 반대다.

코드 줄 수는 263줄로 거의 변하지 않았다. 아니, 조금 더 늘었다.

 

 

그럼 대체 리팩토링을 왜 했냐고 물을 수도 있는데, 

그 의미의 첫 번째는 지저분한 코드들을 항목별로 묶어 함수화하여 가독성을 좋게 만들었다는 것이다.

예를 들어 viewDidLoad() 함수는 아래와 같이 변경됐다.

 

 

위 코드를 보면 대충 어떤 느낌으로 코드가 실행될지 감이 오지 않는가?

카테고리를 설정하고, 로딩서비스의 로딩을 표시한다. 그리고 DelegateDataSource를 세팅하고 Notification을 세팅하고..

카테고리가 뭔지, 로딩서비스가 뭔지 잘 몰라도 일단 어떤 흐름인지 충분히 파악이 된다. 

 

 

이렇게 함수들을 순서대로 써놓으면 아래처럼 함수들을 보기좋게 묶어놓을 수도 있다.

 

 

 

코드양은 오히려 늘었지만 가독성 측면에서 훨씬 나아졌다. 오류가 난 부분을 발견하고 해당 부분을 찾아내고, 언제 실행되는지 알아내고 고치는 데에 좀더 용이해지는 것이다. 물론 이를 위해서는 함수 하나가 한 가지 일만 해야 한다는 것이 전제되어야 한다. (언제나 그럴 수는 없겠지만.)

 

 

결론

파일 하나를 리팩토링하는 것으로 글을 썼기에 예상보다 훨씬 글이 짧았다. 

아무튼 결론을 내리자면 리팩토링은

- 1. 코드양에서 변화를 주는 것이 아니다.

- 2. 가독성을 높이는 행위이다. 

정도로 볼 수 있겠다. 그래서 유지보수하는 것을 좀더 용이하게 만들 수 있다.

 

여기에 더해 디자인 패턴 아키텍쳐를 적용한다면 유지보수하기가 더욱 수월해질 것이다.

 

 

 

* 리팩토링 후의 전체 코드

import UIKit
import Firebase
import CodableFirebase
import CoreLocation


class ListStoreViewController: UIViewController {

    @IBOutlet weak var storeTableView: UITableView!
    @IBOutlet weak var categoryCollectionView: UICollectionView!
    @IBOutlet weak var titleLabel: UILabel!
    
    var ref: DatabaseReference!
    let storage = Storage.storage()
    var categories: [Category] = []
    
    
    var stores = [Store]()
    var filteredStores = [Store]()
    var countForJustExecuteOnce: Bool = false
    
    let refreshControl = UIRefreshControl()

    override func viewDidLoad() {
        super.viewDidLoad()
        
        setCategories()
        LoadingService.showLoading()
        setDelegateDataSource()
        addNotifications()
        self.ref = Database.database(url: "https://restaurants-e62b0-default-rtdb.asia-southeast1.firebasedatabase.app").reference()
        addRefreshControl()
        getStoresInfoFromDatabase()
        setCollectionViewLayout()
    }
    
    override func viewWillAppear(_ animated: Bool) {
        super.viewWillAppear(animated)
        
        setUI() // view will appear에 두는 이유는 다음 화면에서 다시 돌아올 때를 대비하기 위해서.
        adaptFilter()
    }
    
    private func setCategories() {
        self.categories = [Category.all,
                           Category.koreanAsian,
                           Category.japaneseCutlet,
                           Category.chinese,
                           Category.western,
                           Category.chickenPizza,
                           Category.bunsik,
                           Category.mexican,
                           Category.fastFood,
                           Category.meat,
                           Category.cafe]
    }
    
    private func setDelegateDataSource() {
        self.storeTableView.delegate = self
        self.storeTableView.dataSource = self
        self.categoryCollectionView.delegate = self
        self.categoryCollectionView.dataSource = self
    }

    private func addNotifications() {
        NotificationCenter.default.addObserver(self,
                                               selector: #selector(self.didReceiveCategorySelectedNotification(_:)),
                                               name: Notification.Name("categorySelected"), object: nil)
        NotificationCenter.default.addObserver(self,
                                               selector: #selector(self.didReceiveFilterSelectedNotification(_:)),
                                               name: Notification.Name("filterSelected"), object: nil)
    }
    
    @objc func didReceiveCategorySelectedNotification(_ noti: Notification) {
        guard let category = noti.userInfo?["category"] as? String,
              let currentCategory = self.titleLabel?.text
        else { print("can't handle didReceiveCategory Notification"); return; }
        
        if category != currentCategory { executeCategoryFiltering(category) }
        DispatchQueue.main.async { self.titleLabel.text = category }
    }
    
    private func executeCategoryFiltering(_ category: String) {
        switch category {
        case Category.all.rawValue:
            self.filteredStores = self.stores.sorted(by: { $0.storeInfo.createDate < $1.storeInfo.createDate })
        case Category.koreanAsian.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.koreanAsian.rawValue })
        case Category.japaneseCutlet.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.japaneseCutlet.rawValue })
        case Category.chinese.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.chinese.rawValue })
        case Category.western.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.western.rawValue })
        case Category.chickenPizza.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.chickenPizza.rawValue })
        case Category.bunsik.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.bunsik.rawValue })
        case Category.mexican.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.mexican.rawValue })
        case Category.fastFood.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.fastFood.rawValue })
        case Category.meat.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.meat.rawValue })
        case Category.cafe.rawValue:
            self.filteredStores = self.stores.filter({ $0.storeInfo.category == Category.cafe.rawValue })
        default:
            self.filteredStores = self.stores.sorted(by: { $0.storeInfo.createDate < $1.storeInfo.createDate })
            print("in default category filter")
        }
        
        DispatchQueue.main.async {
            NotificationCenter.default.post(name: Notification.Name("categoryDidChange"),
                                            object: nil,
                                            userInfo: nil)
            self.storeTableView.reloadData()
        }
    }
    
    @objc func didReceiveFilterSelectedNotification(_ noti: Notification) {
        guard let filter = noti.userInfo?["filter"] as? String
        else { print("can't handle didReceiveFilter noti"); return }
        
        switch filter {
        case Filter.latest.filterName:
            self.filteredStores = self.filteredStores.sorted(by: { $0.storeInfo.createDate > $1.storeInfo.createDate })
        case Filter.nearest.filterName:
            self.filteredStores = self.filteredStores.sorted(
                by: { self.getDistanceFromCurrentLocation($0.storeInfo.latitude, $0.storeInfo.longtitude) <
                    self.getDistanceFromCurrentLocation($1.storeInfo.latitude, $1.storeInfo.longtitude) })
        case Filter.ratingHigh.filterName:
            self.filteredStores = self.filteredStores.sorted(by: { $0.storeInfo.rating > $1.storeInfo.rating })
        case Filter.reviewCount.filterName:
            self.filteredStores = self.filteredStores.sorted(by: { $0.storeInfo.commentCount > $1.storeInfo.commentCount })
        case Filter.oldest.filterName:
            self.filteredStores = self.filteredStores.sorted(by: { $0.storeInfo.createDate < $1.storeInfo.createDate })
        case Filter.ratingLow.filterName:
            self.filteredStores = self.filteredStores.sorted(by: { $0.storeInfo.rating < $1.storeInfo.rating })
        default:
            self.filteredStores = self.filteredStores.sorted(by: { $0.storeInfo.createDate > $1.storeInfo.createDate })
            print("default filter notification in")
        }
        
        DispatchQueue.main.async { self.storeTableView.reloadData() }
    }

    private func getDistanceFromCurrentLocation(_ targetLatitude: Double, _ targetLongitude: Double) -> CLLocationDistance {
        let currentLocationLatitude = UserDefaults.standard.double(forKey: "currentLocationLatitude")
        let currentLocationLongitude = UserDefaults.standard.double(forKey: "currentLocationLongitude")
        
        let targetLocation = CLLocationCoordinate2D(latitude: targetLatitude, longitude: targetLongitude)
        let currentLocation = CLLocationCoordinate2D(latitude: currentLocationLatitude, longitude: currentLocationLongitude)
        
        return targetLocation.distance(from: currentLocation)
    }
    
    private func addRefreshControl() {
        self.refreshControl.attributedTitle = NSAttributedString(string: "")
        self.refreshControl.addTarget(self, action: #selector(onPullToRefreshFetchDataWhenNoData), for: .valueChanged)
        self.storeTableView.addSubview(refreshControl)
    }
    
    @objc func onPullToRefreshFetchDataWhenNoData() {
        if self.filteredStores.isEmpty == true {
            getStoresInfoFromDatabase()
        }
        DispatchQueue.main.async {
            self.refreshControl.endRefreshing()
        }
    }
    
    private func getStoresInfoFromDatabase() {
        self.ref.child("stores").observe(DataEventType.value, with: { (snapshot) in
            
            if snapshot.exists() {
                self.stores = []
                guard let value = snapshot.value else {return}
                do {
                    let storesData = try FirebaseDecoder().decode([String: StoreInfo].self, from: value)
                    
                    for storeData in storesData {
                        let store: Store = Store(storeKey: storeData.key, storeInfo: storeData.value)
                        self.stores.append(store)
                    }
                    self.filteredStores = self.stores.sorted(by: { $0.storeInfo.createDate < $1.storeInfo.createDate })
                    
                    DispatchQueue.main.async {
                        NotificationCenter.default.post(name: Notification.Name("changeToCurrentFilter"),
                                                        object: nil)
                    }
                } catch let err {
                    print(err.localizedDescription)
                }
            }
        })
        
    }
    
    private func setCollectionViewLayout() {
        if let layout = categoryCollectionView.collectionViewLayout as? UICollectionViewFlowLayout {
            //this value represents the minimum spacing between items in the same column.
            layout.minimumInteritemSpacing = 30
            //this value represents the minimum spacing between successive columns.
            layout.minimumLineSpacing = 30
            layout.sectionInset = UIEdgeInsets(top: 0, left: 20, bottom: 0, right: 20)
        }
    }
    
    private func setUI() {
        self.setStatusBarBackgroundColor()
        self.setNavigationBarBackgroundColor()
        self.storeTableView.backgroundColor = Config.shared.application60Color
        self.setNavigationBarHidden(isHidden: true)
        
    }
    
    private func adaptFilter() {
        NotificationCenter.default.post(name: Notification.Name("changeToCurrentFilter"),
                                        object: nil)
    }
    
}

extension ListStoreViewController: UITableViewDelegate {
    func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
        return self.filteredStores.count
    }
    
    func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        guard let cell = self.storeTableView.dequeueReusableCell(withIdentifier: StoreTableViewCell.reuseIdentifier) as? StoreTableViewCell
        else { return UITableViewCell() }
        
        let store = self.filteredStores[indexPath.row]
        
        cell.store = store
        
        // cell 안에 Firebase를 import하면 용량이 훨씬 커지지 않을까..
        let storageRef = storage.reference()
        let reference = storageRef.child("\(store.storeInfo.mainImage)")
        let placeholderImage = UIImage(named: "placeholder.jpg")
        cell.storeImageView.sd_setImage(with: reference, placeholderImage: placeholderImage)
        
        hideLoadingWhenTableViewDidAppear(indexPath)
        return cell
    }
     
    private func hideLoadingWhenTableViewDidAppear(_ indexPath: IndexPath) {
        if indexPath.row == self.filteredStores.count - 1 { LoadingService.hideLoading() }
    }
}

extension ListStoreViewController: UITableViewDataSource {
    func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
        StoreSingleton.shared.store = self.filteredStores[indexPath.row]
        
        guard let viewController = self.storyboard?.instantiateViewController(withIdentifier: "DetailViewController") as? DetailViewController
        else { return }
            
        tableView.deselectRow(at: indexPath, animated: true)
        self.navigationController?.pushViewController(viewController, animated: true)
        
    }
}