HiveBrain v1.2.0
Get Started
← Back to all entries
patternswiftMinor

Swift menu code

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
swiftcodemenu

Problem

I want to know what do you think about the structure of this code. If you think that there is some structural improvement to be made, tell me!

Here is a github link: https://github.com/LucianoPolit/LPDropdownMenu

Here is the code:

```
//
// LPDropdownMenu.swift
// Created by Luciano Polit on 1/8/16.
// Copyright (c) 2016 Luciano Polit. All rights reserved.
//

import UIKit

@objc protocol LPDropdownMenuViewDelegate {

optional func dropdownMenuView(dropdownMenuView: LPDropdownMenuView, shouldSwipeToPosition position: CGFloat) -> Bool
optional func dropdownMenuView(dropdownMenuView: LPDropdownMenuView, shouldTouchToPosition position: CGFloat) -> Bool
optional func dropdownMenuView(dropdownMenuView: LPDropdownMenuView, shouldDoubleTouchToPosition position: CGFloat) -> Bool
optional func dropdownMenuView(dropdownMenuView: LPDropdownMenuView, shouldAproximateToPosition position: CGFloat) -> Bool
optional func dropdownMenuView(dropdownMenuView: LPDropdownMenuView, shouldFinishSlidingAtPosition position: CGFloat) -> Bool

optional func dropdownMenuView(dropdownMenuView: LPDropdownMenuView, willSwipeToPosition position: CGFloat)
optional func dropdownMenuView(dropdownMenuView: LPDropdownMenuView, willTouchToPosition position: CGFloat)
optional func dropdownMenuView(dropdownMenuView: LPDropdownMenuView, willDoubleTouchToPosition position: CGFloat)
optional func dropdownMenuView(dropdownMenuView: LPDropdownMenuView, willAproximateToPosition position: CGFloat)
optional func dropdownMenuView(dropdownMenuView: LPDropdownMenuView, willFinishSlidingAtPosition position: CGFloat)

optional func dropdownMenuView(dropdownMenuView: LPDropdownMenuView, didSwipeToPosition position: CGFloat)
optional func dropdownMenuView(dropdownMenuView: LPDropdownMenuView, didTouchToPosition position: CGFloat)
optional func dropdownMenuView(dropdownMenuView: LPDropdownMenuView, didDoubleTouchToPosition position: CGFloat)
optional func dropd

Solution

There is a whole lot of code here, but I actually want to focus strictly on the protocol that we've created for delegates of this menu.

There are a few things that immediately jump out to me, but the first thing, I'll illustrate with a screenshot.

The protocol has a total of 17 methods. All of which start with the same 69 characters. Now, that alone is not that problematic. If we take a look at other protocols (even, some of Apple's, like table views and collection views), we'll see the same thing. But here, the problem is, we have provided no comments and the organization of the methods leaves a lot to be desired.

Let's compare, for example, the documentation for UITableViewDelegate:

Notice the organization into more useful categories. Your organization is this:

  • should happen



  • will happen



  • did happen



And then a final "did happen" section that is somewhat inexplicably separated from the other "did happen" methods... but actually makes some as its own section (if we take a look at the Apple organization).

I'd rather see your protocol organized into six section of roughly 3 methods each. The natural organization looks more like this...

  • swiping...



  • should swipe



  • will swipe



  • did swipe



  • touching...



  • should touch



  • will touch



  • did touch



  • etc...



Moreover, I would expect every single on of these method declarations to have appropriate Apple docs with them.

This, for example, is what the shouldSwipeToPosition method might have:

Your methods are well named, but I'm certainly going to expect documentation like this for a delegate protocol. It's the biggest clue I have as to exactly how and when this method works. Your description should even be more detailed than what I've provided.

Consider, for example, the documentation that Apple provides for a similar protocol method found in UIKit:

This documentation is on their website, but it's also available via Xcode in the same manner as I showed in the screenshot demonstrating an example of the documentation you should be providing.

You've dumped everything into a single file. For what you've provided, I'd expect six different files. This means reconsidering your access modifiers.

The thing is, you've designed this with the idea in mind that the user will copy & paste this file into their project and just go... the thing is, that's a terrible way to bring 3rd party code into my project.

When I bring code into my projects, I expect to bring it in via Cocoapods. As a rule (with the rare exception), I completely pass over libraries that aren't Cocoapod compatible.

It's important to note that if our code IS being used via a Cocoapod, then only the things marked as public are seen by the outside world. This is nice. It allows us to access code across our entire library, allowing us to use multiple files, and still preventing the outside world from calling these directly.

It is something you should consider seriously.

Context

StackExchange Code Review Q#116836, answer score: 2

Revisions (0)

No revisions yet.