From b0cee2dee6abfab715bc08fdc1c1606a81f9a75a Mon Sep 17 00:00:00 2001 From: Ivan Smolin Date: Thu, 22 Mar 2018 14:55:43 +0300 Subject: [PATCH] code review notes --- LeadKit.xcodeproj/project.pbxproj | 16 +++++----- .../PaginationWrapper.swift | 32 +++++++++---------- .../DataLoading/RxDataLoadingModel.swift | 11 +++---- ...rapperDelegate+DefaultImplementation.swift | 8 ++--- ...iew.swift => AnyPaginationWrappable.swift} | 2 +- .../PaginationWrapperDelegate.swift | 31 +++++++++--------- 6 files changed, 49 insertions(+), 51 deletions(-) rename Sources/Structures/DataLoading/PaginationDataLoading/{AnyPaginationWrappableView.swift => AnyPaginationWrappable.swift} (97%) diff --git a/LeadKit.xcodeproj/project.pbxproj b/LeadKit.xcodeproj/project.pbxproj index 294ab30d..0d830e52 100644 --- a/LeadKit.xcodeproj/project.pbxproj +++ b/LeadKit.xcodeproj/project.pbxproj @@ -398,9 +398,9 @@ 67EB7FF32061682F00BDD9FB /* TotalCountCursorListingResult+DefaultTotalCountCursorListingResult.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FF02061682F00BDD9FB /* TotalCountCursorListingResult+DefaultTotalCountCursorListingResult.swift */; }; 67EB7FF42061682F00BDD9FB /* TotalCountCursorListingResult+DefaultTotalCountCursorListingResult.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FF02061682F00BDD9FB /* TotalCountCursorListingResult+DefaultTotalCountCursorListingResult.swift */; }; 67EB7FF8206175F700BDD9FB /* PaginationWrappable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FF7206175F700BDD9FB /* PaginationWrappable.swift */; }; - 67EB7FFD206176C900BDD9FB /* AnyPaginationWrappableView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappableView.swift */; }; - 67EB7FFE206176C900BDD9FB /* AnyPaginationWrappableView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappableView.swift */; }; - 67EB7FFF206176C900BDD9FB /* AnyPaginationWrappableView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappableView.swift */; }; + 67EB7FFD206176C900BDD9FB /* AnyPaginationWrappable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappable.swift */; }; + 67EB7FFE206176C900BDD9FB /* AnyPaginationWrappable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappable.swift */; }; + 67EB7FFF206176C900BDD9FB /* AnyPaginationWrappable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappable.swift */; }; 67EB8001206177D600BDD9FB /* PaginationWrapperDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB8000206177D600BDD9FB /* PaginationWrapperDelegate.swift */; }; 67EB8002206177D600BDD9FB /* PaginationWrapperDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB8000206177D600BDD9FB /* PaginationWrapperDelegate.swift */; }; 67EB8003206177D600BDD9FB /* PaginationWrapperDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB8000206177D600BDD9FB /* PaginationWrapperDelegate.swift */; }; @@ -623,7 +623,7 @@ 67EB7FEA2061667900BDD9FB /* DefaultTotalCountCursorListingResult.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultTotalCountCursorListingResult.swift; sourceTree = ""; }; 67EB7FF02061682F00BDD9FB /* TotalCountCursorListingResult+DefaultTotalCountCursorListingResult.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TotalCountCursorListingResult+DefaultTotalCountCursorListingResult.swift"; sourceTree = ""; }; 67EB7FF7206175F700BDD9FB /* PaginationWrappable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaginationWrappable.swift; sourceTree = ""; }; - 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappableView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AnyPaginationWrappableView.swift; sourceTree = ""; }; + 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AnyPaginationWrappable.swift; sourceTree = ""; }; 67EB8000206177D600BDD9FB /* PaginationWrapperDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaginationWrapperDelegate.swift; sourceTree = ""; }; 67FDC25E1FA310EA00C76A77 /* RequestError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RequestError.swift; sourceTree = ""; }; 78405D3B3D3C3E17456877FF /* Pods_LeadKit_iOSTests.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_LeadKit_iOSTests.framework; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -1254,7 +1254,7 @@ 677452BA2062812C0024EEEF /* PaginationDataLoading */ = { isa = PBXGroup; children = ( - 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappableView.swift */, + 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappable.swift */, 67EB8000206177D600BDD9FB /* PaginationWrapperDelegate.swift */, ); path = PaginationDataLoading; @@ -2289,7 +2289,7 @@ A676AE551F98112E001F9214 /* ObservableMappable.swift in Sources */, A6E0DDE11F8A696F002CA74E /* SeparatorRowBox.swift in Sources */, A6E0DDDE1F8A696F002CA74E /* EmptyCellRow.swift in Sources */, - 67EB7FFD206176C900BDD9FB /* AnyPaginationWrappableView.swift in Sources */, + 67EB7FFD206176C900BDD9FB /* AnyPaginationWrappable.swift in Sources */, 671463041EB3396E00EAB194 /* UIView+LoadingIndicator.swift in Sources */, 6774527420624E820024EEEF /* DataLoadingModel.swift in Sources */, 40F118491F8FF223004AADAF /* TableRow+AppearanceExtension.swift in Sources */, @@ -2492,7 +2492,7 @@ 67EB7FD220615B8900BDD9FB /* TotalCountCursorConfiguration.swift in Sources */, EFBE57D31EC35EF20040E00A /* Array+Extensions.swift in Sources */, 6714638B1EB3396E00EAB194 /* RoundDrawingOperation.swift in Sources */, - 67EB7FFF206176C900BDD9FB /* AnyPaginationWrappableView.swift in Sources */, + 67EB7FFF206176C900BDD9FB /* AnyPaginationWrappable.swift in Sources */, A676AE4A1F97D28A001F9214 /* String+Extensions.swift in Sources */, 671463831EB3396E00EAB194 /* PaddingDrawingOperation.swift in Sources */, 6774527C206252020024EEEF /* DataLoadingState.swift in Sources */, @@ -2589,7 +2589,7 @@ 671463491EB3396E00EAB194 /* ResettableType.swift in Sources */, A676AE491F97D28A001F9214 /* String+Extensions.swift in Sources */, 671462E51EB3396E00EAB194 /* UIColor+Hex.swift in Sources */, - 67EB7FFE206176C900BDD9FB /* AnyPaginationWrappableView.swift in Sources */, + 67EB7FFE206176C900BDD9FB /* AnyPaginationWrappable.swift in Sources */, 671462CD1EB3396E00EAB194 /* String+SizeCalculation.swift in Sources */, 677452B720627FE00024EEEF /* PaginationWrappable.swift in Sources */, 67EB7FC1206140E600BDD9FB /* TotalCountCursor.swift in Sources */, diff --git a/Sources/Classes/DataLoading/PaginationDataLoading/PaginationWrapper.swift b/Sources/Classes/DataLoading/PaginationDataLoading/PaginationWrapper.swift index 9299b395..ab81267a 100644 --- a/Sources/Classes/DataLoading/PaginationDataLoading/PaginationWrapper.swift +++ b/Sources/Classes/DataLoading/PaginationDataLoading/PaginationWrapper.swift @@ -29,11 +29,11 @@ final public class PaginationWrapper + private typealias DataLoadingModel = PaginationDataLoadingModel - fileprivate typealias LoadingState = DataLoadingModel.LoadingStateType + private typealias LoadingState = DataLoadingModel.LoadingStateType - private var wrappedView: AnyPaginationWrappableView + private var wrappedView: AnyPaginationWrappable private let paginationViewModel: DataLoadingModel private weak var delegate: Delegate? @@ -71,7 +71,7 @@ final public class PaginationWrapper { + private var stateChanged: Binder { return Binder(self) { base, value in switch value { case .initial: diff --git a/Sources/Classes/DataLoading/RxDataLoadingModel.swift b/Sources/Classes/DataLoading/RxDataLoadingModel.swift index 8babf8a8..29372a7a 100644 --- a/Sources/Classes/DataLoading/RxDataLoadingModel.swift +++ b/Sources/Classes/DataLoading/RxDataLoadingModel.swift @@ -23,12 +23,10 @@ import RxSwift import RxCocoa -open class RxDataLoadingModel: DataLoadingModel - where DLS.DataSourceType: RxDataSource { +open class RxDataLoadingModel: DataLoadingModel + where LoadingStateType.DataSourceType: RxDataSource { - public typealias LoadingStateType = DLS - - public typealias DataSourceType = DLS.DataSourceType + public typealias DataSourceType = LoadingStateType.DataSourceType public typealias ResultType = DataSourceType.ResultType public typealias EmptyResultChecker = (ResultType) -> Bool @@ -69,8 +67,7 @@ open class RxDataLoadingModel: DataLoadingModel } func onGot(error: Error) { - state = .errorState(error: error, - after: state) + state = .errorState(error: error, after: state) } private func onGot(result: ResultType, from dataSource: DataSourceType) { diff --git a/Sources/Extensions/DataLoading/PaginationDataLoading/PaginationWrapperDelegate+DefaultImplementation.swift b/Sources/Extensions/DataLoading/PaginationDataLoading/PaginationWrapperDelegate+DefaultImplementation.swift index 27c12489..6b5c5ac4 100644 --- a/Sources/Extensions/DataLoading/PaginationDataLoading/PaginationWrapperDelegate+DefaultImplementation.swift +++ b/Sources/Extensions/DataLoading/PaginationDataLoading/PaginationWrapperDelegate+DefaultImplementation.swift @@ -75,7 +75,7 @@ public extension PaginationWrapperDelegate { return AnyLoadingIndicator(indicator) } - func retryLoadMoreButton() -> UIButton { + func footerRetryButton() -> UIButton { let retryButton = UIButton(type: .custom) retryButton.backgroundColor = .lightGray retryButton.setTitle("Retry load more", for: .normal) @@ -83,15 +83,15 @@ public extension PaginationWrapperDelegate { return retryButton } - func retryLoadMoreButtonHeight() -> CGFloat { + func footerRetryButtonHeight() -> CGFloat { return 44 } - func retryLoadMoreButtonWillBeShown() { + func footerRetryButtonWillAppear() { // by default - nothing will happen } - func retryLoadMoreButtonWillBeHidden() { + func footerRetryButtonWillDisappear() { // by default - nothing will happen } diff --git a/Sources/Structures/DataLoading/PaginationDataLoading/AnyPaginationWrappableView.swift b/Sources/Structures/DataLoading/PaginationDataLoading/AnyPaginationWrappable.swift similarity index 97% rename from Sources/Structures/DataLoading/PaginationDataLoading/AnyPaginationWrappableView.swift rename to Sources/Structures/DataLoading/PaginationDataLoading/AnyPaginationWrappable.swift index e586f770..a8ace596 100644 --- a/Sources/Structures/DataLoading/PaginationDataLoading/AnyPaginationWrappableView.swift +++ b/Sources/Structures/DataLoading/PaginationDataLoading/AnyPaginationWrappable.swift @@ -23,7 +23,7 @@ import UIKit /// Type that performs some kind of type erasure for PaginationWrappable. -public struct AnyPaginationWrappableView: PaginationWrappable { +public final class AnyPaginationWrappable: PaginationWrappable { private typealias ViewSetter = (UIView?) -> Void diff --git a/Sources/Structures/DataLoading/PaginationDataLoading/PaginationWrapperDelegate.swift b/Sources/Structures/DataLoading/PaginationDataLoading/PaginationWrapperDelegate.swift index d29bbff8..97f181d6 100644 --- a/Sources/Structures/DataLoading/PaginationDataLoading/PaginationWrapperDelegate.swift +++ b/Sources/Structures/DataLoading/PaginationDataLoading/PaginationWrapperDelegate.swift @@ -26,7 +26,7 @@ public protocol PaginationWrapperDelegate: class { associatedtype DataSourceType: DataSource - /// Delegate method that handles loading new chunk of data. + /// Handles loading new chunk of data. /// /// - Parameters: /// - newItems: New items. @@ -34,7 +34,7 @@ public protocol PaginationWrapperDelegate: class { func paginationWrapper(didLoad newItems: DataSourceType.ResultType, using dataSource: DataSourceType) - /// Delegate method that handles reloading or initial loading of data. + /// Handles reloading or initial loading of data. /// /// - Parameters: /// - allItems: New items. @@ -42,12 +42,12 @@ public protocol PaginationWrapperDelegate: class { func paginationWrapper(didReload allItems: DataSourceType.ResultType, using dataSource: DataSourceType) - /// Delegate method that returns placeholder view for empty state. + /// Returns placeholder view for empty state. /// /// - Returns: Configured instace of UIView. func emptyPlaceholder() -> UIView - /// Delegate method that is called when initial loading error is occured. + /// Called when initial loading error is occured. /// It should return true if error is handled and false if it doesn't. /// /// - Parameters: @@ -55,42 +55,43 @@ public protocol PaginationWrapperDelegate: class { /// - Returns: Bool value. If true - then error placeholder wouldn't be shown. func customInitialLoadingErrorHandling(for error: Error) -> Bool - /// Delegate method that returns placeholder view for error state. + /// Returns placeholder view for error state. /// /// - Parameters: /// - error: Error that occured due data loading. /// - Returns: Configured instace of UIView. func errorPlaceholder(for error: Error) -> UIView - /// Delegate method that returns loading idicator for initial loading state. + /// Returns loading idicator for initial loading state. /// This indicator will appear at center of the placeholders container. /// /// - Returns: Configured instace of AnyLoadingIndicator. func initialLoadingIndicator() -> AnyLoadingIndicator - /// Delegate method that returns loading idicator for initial loading state. + /// Returns loading idicator for initial loading state. /// /// - Returns: Configured instace of AnyLoadingIndicator. func loadingMoreIndicator() -> AnyLoadingIndicator - /// Delegate method that returns instance of UIButton for "retry load more" action. + /// Returns instance of UIButton for "retry load more" action. /// /// - Returns: Configured instace of AnyLoadingIndicator. - func retryLoadMoreButton() -> UIButton + func footerRetryButton() -> UIButton - /// Delegate method that returns preferred height for "retry load more" button. + /// Returns height for "retry load more" button. /// - /// - Returns: Preferred height of "retry load more" button. - func retryLoadMoreButtonHeight() -> CGFloat + /// - Returns: Height of "retry load more" button. + func footerRetryButtonHeight() -> CGFloat /// Method is called before "retry load more" will be shown. /// Typically, it's used when you need to show custom footer view. - func retryLoadMoreButtonWillBeShown() + func footerRetryButtonWillAppear() /// Method is called before "retry load more" will be hidden. /// Typically, it's used when you need to hide custom footer view. - func retryLoadMoreButtonWillBeHidden() + func footerRetryButtonWillDisappear() - /// Delegate method, used to clear view if placeholder is shown. + /// Clears view when placeholder is shown. func clearView() + }