This document serves as a location to record high level architectural and implementation patterns, including their histories and any tradeoffs. It is mean to all of us as contributors grow together as we build this app and empower any developer looking to contribute by guiding their efforts through the currently preferred patterns.
It is not meant to explain what the code is doing or how to use specific abstractions. It is also not meant to force us into a dogmatic corner; there are always exceptions to the rules.
This document is to be considered living and is owned by all contributors to this project, be they technical or otherwise. Updates to this document are encouraged as needed and can be submitted via PR as with any other contribution to this repository.
For a detailed discussion of current patterns and implementation within the app see: PATTERNS.md
Table of Contents
This concept is intended to keep code within a file compartmentalized by its intention. The TLDR of this idea is that instead of stacking all of the super types at the top of class, they should be applied via extensions. Furthermore, we prefer to use distinct inheritance over generalization. The first two examples denote
- Clear examples of bad practice A) Stacked Inheritance and B) generalized inheritance
- Example C) denotes leveraging extensions to enhance clarity.
- Example D) gives an example of when we might need to bend the stacked inheritance rule to meet Swift requirements
- Example E) defines an example of how and when we might break with the distinct over generalized rule with good intention
//
// EXAMPLE A) Bad - stacked inheritance
//
class TruckyViewController: UIViewController, UITextViewDelegate, . . . {
...
}
//
// EXAMPLE B) Bad - generalized inheritance
//
class CoolCatTableViewController: UITableViewController {
...
}Instead of stacking extensions or generalizing inheritance we prefer to leverage extensions to enhance clarity. Additionally, we use this pattern as a part of our subscription to the Interface Segregation Principle ie: S.O.L.I.D. This is most clear in the second example C2 below, it is highly possible that our CoolCatViewController may need to provide the DataSource details to a table but may not need to provide Delegate functions, therefore we should not force the CoolCatVC to subscribe to the Delegate portion of the UITableViewController API.
//
// EXAMPLE C) Good - Removing stacked inheritance via extensions
//
class TruckyViewController: UIViewController {
... details specific to UIViewController role
}
extension TruckyViewController: UITextViewDelegate {
... details specific to UITextViewDelegate role
}
//
// EXAMPLE C2) Good - Removing generalized inheritance via extensions
//
class CoolCatViewController: UIViewController {
... details specific to UIViewController role
}
extension CoolCatViewController: UITableViewDataSource {
...
}
extension CoolCatViewController: UITableViewDelegate {
...
}There are times when we may need to bend the rules on stacked inheritance because of requirements associated with Swift or other iOS tools. Take the following example, where we have a protocol that requires subscribers to provide specific properties in addition to functions. Swift does not allow us to declare properties in extensions, and so we are forced to stack our inheritance to meet this demand.
//
// EXAMPLE D) Good - at times it's okay to bend the rules, just do so using best practices
//
protocol DancingInterface: AnyObject {
var isSweatingToTheOldies: Bool { get set }
func danceAllNight()
}
class DancingAllNightViewController: UIViewController, DancingInterface {
...
}Finally, there are times when using a generalized protocol, for example Codable is the best choice, in the following example we have no specific requirements for encoding and decoding and so we use the typealiased Codable interface instead because it clearly defines the features of the struct and provides a succinct implementation.
//
// EXAMPLE E) Good - using a succinct implementation is in alignment with our goal of clarity, and in this case does not violate the ISP of SOLID
struct BigTruck: Codable {
let name: String
let make: String
let model: String
let modelYear: Date
let isAwesome: Bool
}Seek to write code that says clearly what it is doing, without inferring operations or values. In Swift, implicit often means conflation of declaration and instantiation. The strongly typed nature of Swift easily lends itself to very clear syntax, it is important to respect this by clearly defining each step in an objects lifecycle.
Similar to extension over inheritance, the goal here is to keep the code understandable and maintainable in the long run, even if it causes the code to be more verbose. Additionally Swift provides optionality, leveraging explicit over implicit allows us to clearly define this trait.
// implicit, not preferred
let array = [String]()
let number = 10
// explicit, preferred
let array: [String] = []
let number: Int = 10Clear naming can sometimes be verbose, but in the long run helps us more quickly recognize what is happening in any section of code.
- Within reason, prefer clear names over short names
One notable exception to this is in concise blocks where the object acted on is clearly understood and the variable is short lived.
// less clear naming - not preferred
var xfr = ClientTransfer.get(id: 1)
let deads = []
class Data {
...
}
DataStack.dataStack
// clear naming - preferred
var clientTransfer = ClientTransfer.get(id: 1)
let invalidRecords: [Int] = []
class CouponData {
...
}
DataStack.sharedDocumentation of code can be very helpful, however it also has the capacity to detract from the clarity of the code. First and foremost engineers should seek to write clear and concise code which can be easily read and rationalized. However, in addition to taking a pragmatic approach to the quality of your code, it is also helpful to document methodologies using the Codewise documentation tools offered by XCode, this can help you or other developers to quickly rationalize at a high level the purpose and actions of a code block.
It should be generally considered a code smell to leave dangling inline comments throughout code, and should be avoided unless absolutely necessary. That being said, there may be times when in inline comments are the correct choice to add context or clarity for future developers, however they should be develop carefully using a block syntax and provide a complete explanation.
//
// Bad - inline / dangling documentation
//
func canMakeItRain(_ dollars: Int) -> Bool {
dollars.fan() // fan out your scrill for all to see
let rainBucks = dollars x 2 // hustle and double
return dollars.thicc() // determine if you need bigger bands for all those stacks
}
//
// Good - codewise and clear documentation of a func using [Shift + Cmd + /]
//
/// Fan your cash, add some hustle, and see if your new stacks will break bands
/// - Parameter dollars: An Int representing the $$ you currently have before your hustle
/// - Returns: a Bool indicating if you will have mad fat stacks after your hustle
func canMakeItRain(_ dollars: Int) -> Bool {
dollars.fan()
let rainBucks = dollars x 2
return dollars.thicc()
}
//
// Good - codewise block level documentation to add clarity to a cryptic var
//
/**
* This MAX_VIDEO_SIZE number was taken from the Web App.
* There's a little confusion behind this number since we've told clients 200MB is the
* limit, and this is a little off. Our best guess is that it is 200MB + the size of some
* headers or other metadata. All front end applications use this limit, so we will too.
*
* ¯\_(ツ)_/¯
*/
private let MAX_VIDEO_SIZE = 209_715_200
//
// Good - codewise documentation block adding clarity to a strange reaction to a successful API call
//
func bringTheThunder(_ madStacks: [DollaDollaBills], _ completion: (PlayaStatus) -> Void) {
limoService.postUp(madStacks) { result in
switch result {
case .success(let response):
if case response = .wack {
/**
* In the case of a wack status which is still considered to be a valid playa-status we request a retry
* this is because the response returns a 200 despite our app not considering this to be an okay way to play
* the retry will trigger a refresh of the CoreData objects and ask the server to reconsider the status base on the
* most up to data details. We pass the completion to complete the call back cycle when done.
*/
self.retry(completion)
} else {
completion(response)
}
case .failure(let error):
completion(.poser)
}
}
}Storyboards are a tool that while convenient, are not in alignment with craft excellence, and can and have impacted project integrity and the ability to easily expand and maintain the project over time. While there are still currently some storyboards in the app, we have agreed to two conditions going forward.
-
The first is to not introduce any more storyboards to the project, any new views or view controllers should be constructed using UIKit in a codewise manor.
-
The second is to work to clean up and remove storyboards and IB components as the opportunity presents itself and when the time is appropriate. We should not prioritize this effort over the evolution of the product, unless it greatly hinders progress or an opportunity arises to make the effort without disrupting our greater commitment to the product's goals.
Some styles are strictly enforced by SwiftLinters at build time. New linters can be introduced by PR and generally we seek consensus on any new linting rules via conversation on that PR.
Ideally we will keep linting rules specific to allow developers to exercise their judgement, but there are some specific items that we enforce without question. We don't want to implement any processes that distract and hinder developers ability to write code at a high standard. That said, we also expect you to craft your code with care and a personal review before submitting it for review by others.
As a part of our commitment to excellent code and craft we expect developers to introduce no new warnings to the project unless it is required and there is a clear plan for cleaning up the warning at a later date, for example a TODO: - statement that relies on a subsequent PR. Warnings directly impact build performance and thus project integrity.
The app subscribes to a simple principal: CLEAN code == Testable code
In order to support a very high level of testability and thereby a robust code base all testable objects should subscribe to SOLID principles of design. Utilizing interfaces and dependency injection makes mocking and testing objects very easy.
for more details on testing implementation see the Testing Patterns Section in PATTERNS.md
...