Implementing delegate methods in an UIView class

Question

This is my UIView class:

class OnboardingView: UIView {

    @IBOutlet weak var scrollView: UIScrollView!

    @IBOutlet weak var skipButton: UIButton!
    @IBOutlet weak var nextButton: UIButton!
    @IBOutlet weak var pageControl: UIPageControl!

    @IBOutlet weak var skipButtonTopConstraint: NSLayoutConstraint!
    @IBOutlet weak var nextButtonTopConstraint: NSLayoutConstraint!
    @IBOutlet weak var pageControlBottomConstraint: NSLayoutConstraint!

    fileprivate var stepviews: [OnboardingStepViewDelegate] = []

    weak var delegate: OnboardingDelegate?

    fileprivate var visibleIndex: Int = 0 {
        willSet {
                stepviews[visibleIndex].didDisappear()
        }
        didSet {
            stepviews[visibleIndex].didAppear()
        }
    }

    override func awakeFromNib() {
        super.awakeFromNib()

        scrollView.contentSize = CGSize(width: scrollView.frame.width * CGFloat(stepviews.count), height:scrollView.frame.height)
        scrollView.delegate = self

        let firstPage = OnboardingIntroductionStepView.instanceFromNib(title: "Some title", message: "Message", imageName: "page1")

        stepviews = [firstPage, second, third]

        scrollView.contentSize = CGSize(width: 3 * scrollView.frame.size.width, height: scrollView.frame.size.height)

        scrollView.addSubview(firstPage)
    }
}

private extension OnboardingView {

    @IBAction func nextPage(_ sender: Any) {
        if pageControl.currentPage == stepviews.endIndex{
            return
        }

        let nextPageIndex = CGFloat(pageControl.currentPage + 1)

        UIView.animate(withDuration: 0.3) {
            self.scrollView.contentOffset.x = self.scrollView.frame.width * nextPageIndex
        }

        pageControl.currentPage += 1
    }

    @IBAction func skip(_ sender: Any) {
        pageControl.currentPage = stepviews.endIndex
        nextPage(self)
    }

    func moveControlConstraintsOffScreen () {
        pageControlBottomConstraint.constant = 40
        skipButtonTopConstraint.constant = -40
        nextButtonTopConstraint.constant = -40
    }

    func moveControlConstraintsOnScreen () {
        pageControlBottomConstraint.constant = 0
        skipButtonTopConstraint.constant = 16
        nextButtonTopConstraint.constant = 16
    }
}

extension OnboardingView: UIScrollViewDelegate{
    func scrollViewDidScroll(_ scrollView: UIScrollView) {
        self.endEditing(true)

        visibleIndex = Int(scrollView.contentOffset.x / self.frame.width)

        delegate.doSomething()
    }

    func scrollViewWillEndDragging(_ scrollView: UIScrollView, withVelocity velocity: CGPoint, targetContentOffset: UnsafeMutablePointer<CGPoint>) {
        let pageNumber = Int(targetContentOffset.pointee.x / self.frame.width)
        pageControl.currentPage = pageNumber
    }
}

As you can see my view implements the UIScrollViewDelegate. However, this delegate method only handles view related logic. After this view logic is done it will notify the delegate (My ViewController) to do something. Using this approach my ViewController will only contain lifecycle updates (ViewDidLoad, ViewWillDissapear) etc. This makes sense in my opinion - This view class contains a scrollview that implements its logic to do view related tasks, once those tasks are done it tells the VC what to do based on delegation.

My question is: Is this bad and if so, why? Normally MVC is used as the design pattern, which results in "God" ViewControllers where ViewControllers also implement Datasource and Delegate. This results in a ViewController that handles controller logic AND view logic, making the Controller AND View less reusable.

Answer 1

I personally separate my code in my view controllers using the comment //MARK: - which adds shortcuts to your code in the top navigation bar (the one that shows what file you're currently at). I would keep @IB related properties in your original view controller rather than make a extension for the @IBActions.

Also, making an extension for delegates is good practice because it helps separate your code from Apple's delegate methods. I don't see anything wrong with this approach since you can either make an extension to your view, or add it within the original class declaration, but as far as MVC goes, this would be the ideal approach.

As long as you're not doing heavy data/logic/algorithms going on, then you're good.

as for the delegate.doSomething() call, I don't see the point of using delegate since the scroll view delegate code is in an extension of the view, which means doSomething() is already accessible.