0
\$\begingroup\$

So I've been trying to refactor my existing code for loading data from server and synching it to the local DB.
Initially there were multiple methods with duplicated pattern, then I refactored them with generic methods.

Here are both implementations; first approach and second-

First Approach:

private func loadSubFoldersIfNeeded(_ folder: CDFolder, folderType: FolderType, handler: @escaping ([SWGFolder]?) -> Void) {
    if CDSynchronizerHelper.isSyncDisabled() { handler(nil); return }

    self.stack.syncBlock {
        if folder.serverSynchronizationState.intValue == CDObject.NEW { handler(nil); return }

        let syncUserId = self.currentUserId()
        CDSynchronizer.Instance.enterStep()
        self.loadSubFolders(of: folder, folderType: folderType) { remote in
            self.stack.syncBlock {
                if syncUserId!.isEqual(to: self.currentUserId()!) {
                    handler(remote)
                }
            }
            CDSynchronizer.Instance.exitStep()
        }
    }
}

private func loadSubFolders(of folder: CDFolder, folderType: FolderType, handler: @escaping ([SWGFolder]?) -> Void) {
    let folderId = NSNumber(value: folder.folderId)
    switch folderType {
    case .rfi:
        SVApi.Instance.loadRFISubfolders(folderId, handler: handler)
    case .punchList:
        SVApi.Instance.loadPunchListSubfolders(folderId, handler: handler)
    case .dailyReport:
        SVApi.Instance.loadDailyReportSubfolders(folderId, handler: handler)
    case .openIssue:
        SVApi.Instance.loadOpenIssueSubfolders(folderId, handler: handler)
    case .timeEntry:
        SVApi.Instance.loadTimeEntriesSubfolders(folderId, handler: handler)
    default:
        preconditionFailure("folderType case must be implemented")
        print("switch exausted...")
    }
}

Example usage:

func syncPunchListSubfolders(_ _folder: CDFolder, handler: @escaping () -> Void) {
    let folder = _folder.forEdit(self.stack)
    loadSubFoldersIfNeeded(_folder, folderType: .punchList) { remote in
        let collection = CDPunchListFolderCollection(stack:self.stack, project: folder.refProject, parentFolder: folder)
        collection.processSync(remote)
        handler()
    }
}

So all the other folder types were having their own implementation like above for calling loadSubFoldersIfNeeded method.

Second Approach:

In this approach I'm most concerned by usage of blocks. Two different black are used in generic method-
1 for loading data from server
2 returning processed data

private typealias LoadFoldersCompletionBlock = ([SWGFolder]?) -> Void
private typealias LoadFoldersRequetBlock = (@escaping LoadFoldersCompletionBlock) -> Void
private func loadSubFoldersIfNeeded(_ folder: CDFolder,
                              requestHandler: @escaping LoadFoldersRequetBlock,
                              completionHandler completion: @escaping LoadFoldersCompletionBlock) {

    if CDSynchronizerHelper.isSyncDisabled() { completion(nil); return }

    self.stack.syncBlock {
        if folder.serverSynchronizationState.intValue == CDObject.NEW { completion(nil); return }

        let syncUserId = self.currentUserId()
        CDSynchronizer.Instance.enterStep()
        requestHandler() { remote in
            self.stack.syncBlock {
                if syncUserId!.isEqual(to: self.currentUserId()!) {
                    completion(remote)
                } else {
                    completion(nil)
                }
            }
            CDSynchronizer.Instance.exitStep()
        }
    }
}

Example usage:

func syncPunchListSubfolders(_ _folder: CDFolder, handler: @escaping () -> Void) {
    let folder = _folder.forEdit(self.stack)
    let folderId = NSNumber(value: _folder.folderId)
    loadSubFoldersIfNeeded(folder, requestHandler: { requestCompletion in
        SVApi.Instance.loadPunchListSubfolders(folderId) { remote in
            requestCompletion(remote)
        }
    }, completionHandler: { remote in
        let collection = CDPunchListFolderCollection(stack:self.stack, project: folder.refProject, parentFolder: folder)
        collection.processSync(remote)
        handler()
    })
}

All other folder types has to implement same method like above for each to call loadSubFoldersIfNeeded.

Original Implementation:

func syncPunchListSubfolders(_ _folder: CDFolder, handler: @escaping () -> Void) {
    if CDSynchronizerHelper.isSyncDisabled(handler) { return }

    self.stack.syncBlock({
        if _folder.serverSynchronizationState.intValue == CDObject.NEW {
            return
        }

        let folder = _folder.forEdit(self.stack)
        let syncUserId = self.currentUserId()

        CDSynchronizer.Instance.enterStep();
        SVApi.Instance.loadPunchListSubfolders(NSNumber(value: folder.folderId), handler: { remote in
            self.stack.syncBlock({
                if syncUserId!.isEqual(to: self.currentUserId()!) {
                    let collection = CDPunchListFolderCollection(stack:self.stack, project: folder.refProject, parentFolder: folder)
                    if remote != nil {
                        collection.processSync(remote)
                    }
                }

                handler()
            })

            CDSynchronizer.Instance.exitStep()
        })
    })
}

This same method was duplicated for all folder types, expect from SVApi.Instance.loadPunchListSubfolders call to load folders from server and synch data in CDPunchListFolderCollection DB class.

Please tell me which approach is best (recommended), first or second?
Also point if I'm doing anything wrong in refactoring.
Even if you have any different approach for refactoring that would be welcomed...

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.