Clean Code Principles

Functions

Principle 1. A name of a function should tell what it does without looking at the implementation

// It is a real function name from the project I participated in
func checkNoMovementDuringAction(
element: Element,
action: () -> Void
)
// What is the movement?
// And what happens if that movement occurs? I'd suggest a bool
// Why do we need an element here? Is it about the movement?
// Let's rename it
func doActionAndThrowErrorIfElementPositionIsChanged(
action: () -> Void,
element: Element
)
// Well, now the name answers all questions above,
// but it's quite long, right?
// And it tells implementations details like error throwing,
// which is not good
// Let's change its name again
func doActionAndEnsureElementPositionNotChanged(
action: () -> Void,
element: Element
)
// I think this variant is appropriate
// It's a kind of compromise between expressiveness and length
// 'ensure' describes things better than 'check' in this case
// And the name is abstract enough
let result = accountRepository.update(account)
// What is the result? .success? updatedAccount? Both?
// And what if I don't need anything but just updating?
// I tried to refactor
accountRepository.update(account)
let updatedAccount = accountRepository.getBy(id: account.id)
...// However, the worst thing is when you expect the function
// just to return a value, but inside it updates the database
let accounts = accountService.createAccountsFrom(accountDTOs)
// createAccountsFrom uses accountRepository to save accounts
// I would suggest the following
let accounts = accountService.createAccountsFrom(accountDTOs)
accountService.saveAccounts(accounts)
// Or even rename 'create' to 'build' to avoid possible confusions
let accounts = accountService.buildAccountsFrom(accountDTOs)
accountsService.saveAccounts(accounts)
// And one more piece of refactoring here ;)
let accounts = accountFactory.buildAccountsFrom(accountDTOs)
accountService.saveAccounts(accounts)
// ... perfect
  • Tests become easier
  • It doesn’t take many pieces of knowledge to use
  • It gets simpler to understand how it works
// Bad
func start() {
initializeResources()
let topics = getTopicsFrom(config)
for topic in topics {
consumer.subscribe(topic: topic, fromBeginning: true)
}
runKafkaConsumer()
let logBody = [
"tenantAlias": getTenantAlias(),
"consumerGroup": getConsumerGroup(),
"kafkaHost": config.kafkaHost,
"cluster": config.cluster,
"deploymentType": config.deploymentType,
]
logger.info(logBody)

}
// Good
func start() {
initializeResources()
subscribeToTopicsFromConfig()
runKafkaConsumer()
logKafkaConsumerStarted()
}
// Bad
func loadView() {
createSegmentedControlView()
self.tableView = createTableView()
createRootView(self.segmentedControlView, self.tableView)
}
// Good
func loadView() {
self.segmentedControlView = createSegmentedControlView()
self.tableView = createTableView()
self.view = createRootView(
segmentedControlView: self.segmentedControlView,
tableView: self.tableView
)
}
// or
func loadView() {
createSegmentedControlView()
createTableView()
createRootView()
}
// However, this variant is worse
// because the order of function calls here is important
// (temporal binding)
// Bad
function setupSegmentedControlView() {
createSegmentedControlView()
setSegmentedControlConstraints()
}
// If setting constraints of the control stays first,
// it will produce a runtime error
// Good
func setupSegmentedControlView() {
self.segmentedControlView = createSegmentedControlView()
setConstraintsOf(segmentedControl: self.segmentedControlView)
}
// It's impossible to call setting constraints
// before the control is created
// Bad
dictionary[regexpMatch.group(0)] = regexpMatch.group(1)
// Good
let id = regexpMatch.group(0)
let name = regexpMatch.group(1)
dictionary[id] = name
// Bad
if timer.isExpired && !timer.isRecurrent { ... }
// Good
class Timer {
var shouldBeDeleted: Bool {
return isExpired && !isRecurrent
}
}
...
if timer.shouldBeDeleted { ... }
// or
func shouldTimerBeDeleted(_ timer: Timer) -> Bool {
return timer.isExpired && !timer.isRecurrent
}
...
if shouldTimerBeDeleted(timer) { ... }
// This example uses JavaScript (btw, other examples use Swift)
// ... Swift is the best! ;)
// Bad
async function performRequest(...) {
try {
const endpoint = createEndpoint(...)
const body = createRequestBody(...)
const request = createRequest(endpoint, body)
await request.perform()
} catch {
console.log("Oops. Failed request")
}
}
// Good
async function performRequest(...) {
const endpoint = createEndpoint(...)
const body = createRequestBody(...)
const request = createRequest(endpoint, body)
await request.execute()
}
async function tryToPerformRequest(...) {
try {
performRequest(...)
} catch {
console.log("Oops. Failed request")
}
}

Classes, Structures, and Other Custom Types

Principle 1. A class name should stick to the standard naming

class MyCuteBrowser {
let browser: WebBrowser

func open(url: URL?) { ... }
}
let myCuteBrowser = MyCuteBrowser()
myCuteBrowser.open(url: URL("http://google.com"))
// Bad
myCuteBrowser.browser.reloadCurrentPage() // Transitive dependency
// Good
class MyCuteBrowser {
private browser: WebBrowser
...
func reloadCurrentPage() {
browser.reloadCurrentPage()
}
}
...
myCuteBrowser.reloadCurrentPage()
// There can be used the principle ‘put the code next to its usage’
class ViewController: UIViewController {
override func viewDidLoad() {
super.viewDidLoad()
setupTableView()
setupBottomButton()
}
private setupTableView() { ... }
private setupBottomButton() { ... }
override func updateViewConstraints() {
super.updateViewConstraints()
...
}
}
// internal private private, internal
...// Or there can be a single convention about
// where functions and properties should be placed
class ViewController: UIViewController {
// MARK: - Lifecycle
override func viewDidLoad() {
super.viewDidLoad()
setupTableView()
setupBottomButton()
}
override func updateViewConstraints() {
super.updateViewConstraints()
...
}
// MARK: - Private methods
private setupTableView() { ... }
private setupBottomButton() { ... }
}

General

Principle 1. The same word should have just one meaning. And only this word should be used everywhere when talking about the same thing. There should be no other synonyms

  • Having an ability to quickly replace one library with another without changing the rest code (rare case, but still)
  • Picking an interface better for your needs (you can choose another name of a method or even combine several methods to a single one)
  • Writing tests for the wrappers that will tell your teammates how they are used
// Bad
class MyCuteBrowser {
private let browser = WebBrowser()
...
}
// Good
class MyCuteBrowser {
private let browser: WebBrowser

init(browser: WebBrowser) {
self.browser = browser
}
}
class MyCuteObjectFactory {
...
func createMyCuteBrowser() {
return MyCuteBrowser(browser: WebBrowser())
}
}
// Bad
func tableView(..., willDisplay cell: UITableViewCell, ...) {
let entity = entities[indexPath.row]
switch entity {
case is Movie: ...
case is PopularPerson: ...
default: ...
}
}
func tableView(..., cellForRowAt indexPath: IndexPath) -> ... {
let entity = entities[indexPath.row]
switch entity {
case is Movie: ...
case is PopularPerson: ...
default: ...
}
}
// Good
func tableView(..., willDisplay cell: UITableViewCell, ...) {
let entity = entities[indexPath.row]
getEntityUtilsFactory(by: entity)
.setMultiSearchTableViewCellImageProperties(
cell: cell,
by: indexPath
)
}
func tableView(..., cellForRowAt indexPath: IndexPath) -> ... {
let entity = entities[indexPath.row]
return getEntityUtilsFactory(by: entity)
.getMultiSearchTableViewCell(
from: tableView,
by: indexPath
)
}
func getEntityUtilsFactory(
by entity: MultiSearchEntity
) -> EntityUtilsAbstractFactory {
switch entity {
case is Movie: return movieUtilsFactory
case is PopularPerson: return personUtilsFactory
default: fatalError("Entity type is unexpected.")
}
}
class EntityUtilsAbstractFactory {
func setMultiSearchTableViewCellImageProperties(
cell: UITableViewCell,
by indexPath: IndexPath
) { }
func getMultiSearchTableViewCell(
from tableView: UITableView,
by indexPath: IndexPath
) -> UITableViewCell {
fatalError("getTableViewCell(..) must be overridden.")
}
}
// Let's imagine you have a type Person
struct Person {
let name: String
let age: Int
}
// And you decided to create a greeting generator for people
class PersonGreetingGenerator {
// And you said: "Oh, it's always enough to have just a name
// to create a greeting"
// You decided that a person name is enough to use your code
func createGreetingFor(personName: String) -> String {
return "Hello, \(personName)!"
}
}
// And what's wrong?// Okay. Let's imagine that we have the following function
func greet(
person: Person,
via generator: PersonGreetingGenerator
) {
let greeting = generator.createGreetingFor(
personName: person.personName
)
print(greeting)
}
// And we want to generate a special greeting for old people
class AgeSpecificPersonGreetingGenerator: PersonGreetingGenerator {
// Well, now it's not enough to have just a name
...
// Should we create one more function like this?
func createGreetingFor(
personName: String,
personAge: Int
) -> String {
...
}
}
// But how can we use it with different generators?
greet(person: person, via: defaultGenerator)
greet(person: oldPerson, via: ageSpecificGenerator)
// Should we create one more function? Or add switch/case there?
// No// We shouldn't decide what person fields should be used
// We just say: "A person greeting generator works with a person"
class PersonGreetingGenerator {
func createGreetingFor(person: Person) -> String {
return "Hello, \(person.name)!"
}
}
class AgeSpecificPersonGreetingGenerator: PersonGreetingGenerator {
override
func createGreetingFor(person: Person) -> String {
...
}
}
func greet(
person: Person,
via generator: PersonGreetingGenerator
) {
let greeting = generator.createGreetingFor(person: person)
print(greeting)
}
greet(person: person, via: defaultGenerator)
greet(person: oldPerson, via: ageSpecificGenerator)

P.S.

First of all, I would like to say thanks to Robert Martin and Eric Evans. Almost everything in this article I took from ‘Clean Code’ and ‘Domain-Driven Design’ I’m a big fan of those guys!

  • I didn’t manage to put some of them into practice
  • For now, I’m quite stupid to understand some others
  • Some of them I simply might miss while reading
  • And others are obvious (follow SOLID, keep modularity, comments are bad, …)

Another P.S.

I think it’s wrong to follow each principle in 100% of cases

And The Last P.S.

Keep your code straightforward. Don’t try to use elaborate solutions with generic types, tons of abstractions, etc. People call it ‘overengineering’

References

  • Clean Code: A Handbook of Agile Software Craftsmanship, by Robert C. Martin
  • Domain-Driven Design: Tackling Complexity in the Heart of Software, by Eric Evans

--

--

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store