-
Notifications
You must be signed in to change notification settings - Fork 0
Code Style Guidelines
When writing code in Cascable projects, we favour clarity and explicitness over shorthand and brevity. The goal is to write code that's reasonably self-describing and understandable to people who are new to the codebase and the domain at hand.
Reducing lines of code through use of shorthand and hard-to-understand functions is not a goal here.
Code should be set to indent with spaces at 4 spaces per indentation level. Aim to have individual lines of code be less than 120 characters long including indentation, but it's OK to overshoot a little occasionally. Enable your text editor's page guide at column 120 to help keep this in check.
Always be expressive and use full words when naming classes, functions, parameters, and variables. Variable names should describe what they contain but generally not the data type in the name itself. Class and function names should describe what they do in a specific manner rather than a generic manner (i.e., URLSanitiser would be a better name than StringProcessor for a class that processes strings for use in URLs). Also avoid "filler" words like "data" etc - they don't add anything.
Note: When declaring variables, explicitly declaring the variable's type is favoured. This helps with code readability and clarity, and can have the side effect of improving Swift compile times — particularly in SwiftUI contexts. This isn't a hard and fast rule — omitting the type name is acceptable when there's arguably no confusion about what the type could be.
let camStr = camera.modelName // Bad - "camStr" is not descriptive
let cameraName = camera.modelName // OK
let cameraName: String = camera.modelName // Best
let generator: OpportunityGenerator = OpportunityGenerator(with: engines) // Redundant type declarations not needed
let generator = OpportunityGenerator(with: engines) // Good
// OK since the type can be easily be inferred by the name, even though the name doesn't contain the type
let cameraNameIsLong = (cameraName.count > 20)
// This is still better
let cameraNameIsLong: Bool = (cameraName.count > 20)
func process(_ str: String) -> String // Bad - "process" doesn't tell us anything
func removeSpaces(from input: String) -> String // GoodThe phrase "code should be self-documenting" is often uttered in style guides, and that's a good aim — these guidelines are indeed set out with the goal of making code as understandable as it can be to developers not already familiar with it. However, while good code can make it easy to understand what it's doing (and as such is "self-documenting" in that manner), it often can't say why. This is where good commenting etiquette comes in.
It's hard to make an explicit rule about when to do this, but if you write some code and think you're likely to immediately get asked "Why did you do that?" by someone else looking at the code for the first time, that's a good point to add a comment explaining why — especially if there's a ticket asking for the desired behaviour.
This is a bad example of a code comment - it doesn't add anything over what the code around it is telling us:
let productLevelForWeatherLookup: PhotoScoutAPI.ProductLevel = try await {
// Always grant the full amount of weather data if the request is within 10 metres of
// the Instant Inspiration demo coordinate.
let instantInspirationDemoCoordinate = Coordinate(latitude: 59.33041, longitude: 18.05882)
let distanceFromDemo = requestBody.location.distance(from: instantInspirationDemoCoordinate)
if distanceFromDemo < Reading<DistanceUnit>(value: 10.0, unit: .meters) {
return .full
}
…
}Instead, we should explain why we're granting the full amount of weather data to that location. This is a good example of a code comment:
let productLevelForWeatherLookup: PhotoScoutAPI.ProductLevel = try await {
// PS-599: Users on an expired trial are allowed to use a demo version of the Instant Inspiration feature,
// which is locked to a location in Stockholm. For this demo to put its best foot forward, we should provide
// the best weather data we can.
let instantInspirationDemoCoordinate = Coordinate(latitude: 59.33041, longitude: 18.05882)
let distanceFromDemo = requestBody.location.distance(from: instantInspirationDemoCoordinate)
if distanceFromDemo < Reading<DistanceUnit>(value: 10.0, unit: .meters) {
return .full
}
…
}In short, code comments shouldn't make up for hard-to-read code — they should add context that the code can't necessarily provide.
It can be tempting to chain operations together to avoid declaring "temporary" storage variables, but well-named variables can add a lot of context and clarity to your code. If adding such a variable aids readability, add it.
For example, it's not clear what this code is doing:
if requestBody.location.distance(from: Coordinate(latitude: 59.33041, longitude: 18.05882)) <
Reading<DistanceUnit>(value: 10.0, unit: .meters) {
return .full
}Adding some intermediate variables adds clarity:
let instantInspirationDemoCoordinate = Coordinate(latitude: 59.33041, longitude: 18.05882)
let distanceFromDemo = requestBody.location.distance(from: instantInspirationDemoCoordinate)
if distanceFromDemo < Reading<DistanceUnit>(value: 10.0, unit: .meters) {
return .full
}Swift allows closure parameters at the end of function parameter lists to be omitted by name. However, especially if the closure isn't called at an extremely obvious point explained by surrounding context, they should be avoided and the parameter name included.
let coordinator = CameraPropertyCoordinator(for: .exposureCompensation) {
// When is this called??
} Adding the parameter name gives context to when the closure will be called:
let coordinator = CameraPropertyCoordinator(for: .exposureCompensation, valueChangedHandler: {
// It's called when the value changes!
})SwiftUI loves trailing closure syntax, and it's very commonly used there. As long as the code remains readable and understandable, it's OK to use trailing closure syntax in SwiftUI context. Multiple trailing closures should still be avoided.
Shorthand parameters in Swift (i.e., $0, $1, etc) can reduce clutter when used carefully, but it's very easy to lose important context when using them. They should only be used when doing so doesn't harm readability of the code for someone not familiar with it. A general rule is that if you're starting to use $1, $2, and beyond you should switch to explicit parameter declarations.
This is an example of a hard to understand section of code:
robot.publisher(for: \.statusGenerator.currentShotRemainingCount)
.combineLatest(robot.publisher(for: \.currentModule.estimatedNumberOfShots))
.combineLatest(robot.publisher(for: \.currentRun).compactMap({ $0 != nil }))
.flatten()
.sink(receiveValue: {
currentLayout.noteShutterRobotRunUpdatedShotCount($2 ? $0 : $1, isInActiveRun: $2)
})Not using shorthand parameters makes the code much clearer:
robot.publisher(for: \.statusGenerator.currentShotRemainingCount)
.combineLatest(robot.publisher(for: \.currentModule.estimatedNumberOfShots))
.combineLatest(robot.publisher(for: \.currentRun).compactMap({ $0 != nil }))
.flatten()
.sink(receiveValue: { shotsRemainingInRun, currentModuleShotEstimate, hasActiveRun in
let shotCountToDisplay = hasActiveRun ? shotsRemainingInRun : currentModuleShotEstimate
currentLayout.noteShutterRobotRunUpdatedShotCount(shotCountToDisplay, isInActiveRun: hasActiveRun)
})Simple, clear, and common uses of shorthand are OK to use, as long as the line as a whole is clear and readable.
let cameraNames = cameras.map({ $0.modelName })
let firstEOS = cameraNames.first(where: { $0.hasPrefix("EOS") })Similar to shorthand parameters, unnamed tuple values should also be avoided. Always declare labels for tuple values.
This is an example of a hard to understand section of code:
let cameraDetails: (String, String, String) = […]
let cameraModelsByFriendlyName = cameraDetails.reduce(into: [:], { $0[$1.2] = $1.1 })Adding tuple labels makes the code much clearer:
let cameraDetails: [(manufacturer: String, model: String, friendlyName: String)] = […]
let cameraModelsByFriendlyName = cameraDetails.reduce(into: [:], { result, details in
result[details.friendlyName] = details.model
})In general, if you end up with a tuple with several values you should consider creating a struct instead.
Swift allows single-line functions that return values to omit the return statement. This behaviour is fragile (the behaviour of a line of code relying on an implicit return will change based on unrelated changes above it) and prone to side effects and misunderstandings, and should be avoided.
Avoid this:
func stripSpaces(from input: String) -> String {
input.replacingOccurrences(of: " ", with: "")
}
var welcomeString: String {
"Hello!"
}Do this:
func stripSpaces(from input: String) -> String {
return input.replacingOccurrences(of: " ", with: "")
}
var welcomeString: String {
return "Hello!"
}Simple, clear, and common uses of implicit return statements are OK to use as long as the line as a whole is clear and readable.
let cameraNames = cameras.map({ $0.modelName })
let firstEOS = cameraNames.first(where: { $0.hasPrefix("EOS") })While operator precedence is generally well-defined, it's preferred that brackets are added to keep things explicit, particularly if chained operators look similar or aren't well understood by everybody. Some examples:
let numberIsBig: Bool = value > 1000 // Acceptable: not a lot of confusion here
let numberIsBig: Bool = (value > 1000) // Better!
let numberIsAThousand: Bool = value == 1000 // Discouraged: = and == look very similar
let numberIsAThousand: Bool = (value == 1000) // Good
let valueIsNumeratorFlag: UInt8 = value & 0xff000000 >> 24 // Bad: bitwise operators aren't well-known by everybody
let valueIsNumeratorFlag: UInt8 = ((value & 0xff000000) >> 24) // GoodThe purpose of these guidelines is to create code that's clear and understandable to everyone - both people not familiar with the code and the original author months down the line. When in doubt, always favour the option that adds context and clarity.