I’m slowly making my way through the Swift track on Exercism.io and hit the Scale Generator exercise.
Most of my time was spent trying to understand the requirements from the description. I finally downloaded the source from github and could see what was going on—some missing Markdown made the resulting HTML unclear. I filed a PR and hope it gets merged at some point. In the meantime, here is my cleaned up version in case it helps.
I had a lot of fun with this exercise and wanted to walk through what a noob like me went through to refactor from “solving the problem” to a “well-engineering Swifty solution” (caveat: I am still learning what “Swifty” means and have certainly missed many opportunities to leverage the standard library or other resources to solve this exercise. I hope that the approach I used is useful in spite of that.)
1st Working Solution
My first working solution looked like this:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
|
import Foundation
struct ScaleGenerator {
let tonic: String
let scale: String
let pattern: String
let sharps = ["A", "A#", "B", "C", "C#", "D", "D#", "E", "F", "F#", "G", "G#"]
let flats = ["A", "Bb", "B", "C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab"]
typealias Scale = [String]
init(tonic: String, scaleName: String, pattern: String = "mmmmmmmmmmm") {
self.tonic = tonic
self.scale = scaleName
self.pattern = pattern
}
var name: String {
return tonic.capitalized + " " + scale
}
func pitches() -> Scale {
switch tonic {
case "C", "a",
"G", "D", "A", "E", "B", "F#", "e", "b", "f#", "c#", "g#", "d#":
let start = sharps.firstIndex(of: tonic.capitalized) ?? 0
let tonicScale = Array(sharps[start...] + sharps[0..<start])
let intervals = patternIntervals()
var result = Scale()
for interval in intervals {
if (interval > tonicScale.endIndex) {
break
}
result.append(tonicScale[interval])
}
return result
case "F", "Bb", "Eb", "Ab", "Db", "Gb", "d", "g", "c", "bb", "eb":
let start = flats.firstIndex(of: tonic.capitalized) ?? 0
let tonicScale = Array(flats[start...] + flats[0..<start])
let intervals = patternIntervals()
var result = Scale()
for interval in intervals {
if (interval > tonicScale.endIndex) {
break
}
result.append(tonicScale[interval])
}
return result
default:
return sharps
}
}
internal func patternIntervals() -> [Int] {
var index = 0
var interval = [0]
for int in pattern {
switch int {
case "M":
index += 2
case "m":
index += 1
case "A":
index += 3
default:
break
}
if index % 12 < index { // FIXME: depends on scale length
continue
}
interval.append(index)
}
return interval
}
}
|
I imported Foundation to get the capitalized property. You can see that the pitches() method is just way too long and has repeating code blocks. The patternIntervals() method is also long and bakes values in the switch statement the method that will make it fragile to changes. But it “works” and all the tests pass.
2nd Working Solution
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
|
import Foundation
struct ScaleGenerator {
let tonic: String
let scale: String
let pattern: String
let sharps = ["A", "A#", "B", "C", "C#", "D", "D#", "E", "F", "F#", "G", "G#"]
let flats = ["A", "Bb", "B", "C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab"]
typealias Scale = [String]
init(tonic: String, scaleName: String, pattern: String = "mmmmmmmmmmm") {
self.tonic = tonic
self.scale = scaleName
self.pattern = pattern
}
var name: String {
return tonic.capitalized + " " + scale
}
var intervals: [Int] {
var index = 0
let intervals: [Character: Int] = ["m": 1, "M": 2, "A": 3,]
return [0] + pattern.compactMap { intervals[$0] }
.map { (int: Int) -> Int in
index += int
return index
}.filter { $0 <= $0 % sharps.count }
}
func pitches() -> Scale {
switch tonic {
case "C", "a",
"G", "D", "A", "E", "B", "F#", "e", "b", "f#", "c#", "g#", "d#":
let start = sharps.firstIndex(of: tonic.capitalized) ?? 0
let tonicScale = Array(sharps[start...] + sharps[0..<start])
var result = Scale()
for interval in intervals {
if (interval > tonicScale.endIndex) {
break
}
result.append(tonicScale[interval])
}
return result
case "F", "Bb", "Eb", "Ab", "Db", "Gb", "d", "g", "c", "bb", "eb":
let start = flats.firstIndex(of: tonic.capitalized) ?? 0
let tonicScale = Array(flats[start...] + flats[0..<start])
var result = Scale()
for interval in intervals {
if (interval > tonicScale.endIndex) {
break
}
result.append(tonicScale[interval])
}
return result
default:
return sharps
}
}
}
|
In this revision, I pulled out the intervals into a calculated property and compacted it with some functional techniques. I have mixed feelings about this property, but in the end I didn’t do much more to change this.
3rd Working Solution
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
|
import Foundation
struct ScaleGenerator {
let tonic: String
let scale: String
let pattern: String
let sharps = ["A", "A#", "B", "C", "C#", "D", "D#", "E", "F", "F#", "G", "G#"]
let flats = ["A", "Bb", "B", "C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab"]
typealias Scale = [String]
init(tonic: String, scaleName: String, pattern: String = "mmmmmmmmmmm") {
self.tonic = tonic
self.scale = scaleName
self.pattern = pattern
}
var name: String {
return tonic.capitalized + " " + scale
}
var intervals: [Int] {
var index = 0
let intervals: [Character: Int] = ["m": 1, "M": 2, "A": 3]
return [0] + pattern.compactMap { intervals[$0] }
.map { (gap: Int) -> Int in
index += gap
return index
}.filter { $0 <= $0 % sharps.count }
}
func pitches() -> Scale {
switch tonic {
case "C", "a",
"G", "D", "A", "E", "B", "F#", "e", "b", "f#", "c#", "g#", "d#":
return generatePitches(sharps)
case "F", "Bb", "Eb", "Ab", "Db", "Gb", "d", "g", "c", "bb", "eb":
return generatePitches(flats)
default:
return sharps
}
}
internal func generatePitches(_ someScale: Scale) -> Scale {
let start = someScale.firstIndex(of: tonic.capitalized) ?? 0
let tonicScale = Array(someScale[start...] + someScale[0..<start])
return intervals.map { tonicScale[$0] }
}
}
|
In this revision I pull out that duplicated code in the pitches() section into its own method. I also have some unit tests for it in the test file. It’s starting to get “clean” with small, testable methods and good tests.
4th Working Solution
At this point I took a drive to clear my head and think about the code: what would make this more “Swifty”? While I’m just beginning to get a sense of this, I see extensions, typealiases, and protocols as language features that allows cleaner expression and reusability.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
|
import Foundation
extension Array where Element: Equatable {
func rotated(from: Element) -> [Element] {
let start = self.firstIndex(of: from) ?? startIndex
return Array(self[start...] + self[0..<start])
}
}
struct ScaleGenerator {
typealias Scale = [String]
let tonic: String
let mode: String
let pattern: String
let sharps: Scale = ["A", "A#", "B", "C", "C#", "D", "D#", "E", "F", "F#", "G", "G#"]
let flats: Scale = ["A", "Bb", "B", "C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab"]
init(tonic: String, scaleName: String, pattern: String = "mmmmmmmmmmm") {
self.tonic = tonic
self.mode = scaleName
self.pattern = pattern
}
var name: String {
return tonic.capitalized + " " + mode
}
var intervals: [Int] {
var index = 0
let intervals: [Character: Int] = ["m": 1, "M": 2, "A": 3]
return [0] + pattern.compactMap { intervals[$0] }
.map { (gap: Int) -> Int in
index += gap
return index
}.filter { $0 <= $0 % sharps.count }
}
var baseScale: Scale {
switch tonic {
case "F", "Bb", "Eb", "Ab", "Db", "Gb", "d", "g", "c", "bb", "eb":
return flats
default: // "C", "a", "G", "D", "A", "E", "B", "F#", "e", "b", "f#", "c#", "g#", "d#":
return sharps
}
}
func pitches() -> Scale {
let tonicScale = baseScale.rotated(from: tonic.capitalized)
return intervals.map { tonicScale[$0] }
}
}
|
I now extend the Array type and add a rotated method that returns the array “rotated” at the given from: element. Here’s how it works:
1
2
3
4
5
6
7
8
9
|
extension Array where Element: Equatable {
func rotated(from: Element) -> [Element] {
let start = self.firstIndex(of: from) ?? startIndex
return Array(self[start...] + self[0..<start])
}
}
["A", "B", "C", "D", "E", "F", "G"].rotated(from: "D")
// return ["D", "E", "F", "G", "A", "B", "C"]
|
This simplifies pitches() further and gives us some clues on how to clean it up further.
5th Working Solution
The last major revision I made is purely syntactic sugar: I wanted to make the little map over intervals in the pitches() method more obvious what is happening, so I extended Array again with a pick method, and I removed the dependency on Foundation with an extension on StringProtocol I found on stackoverflow (I later found a similar extension by Paul Hudson on his site, but the SO link is older and I found it first there):
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
|
// https://stackoverflow.com/a/28288340
extension StringProtocol {
var firstUppercased: String { prefix(1).uppercased() + dropFirst() }
}
extension Array where Element: Equatable {
func rotated(from: Element) -> [Element] {
let start = self.firstIndex(of: from) ?? startIndex
return Array(self[start...] + self[0..<start])
}
func pick(from: [Int]) -> [Element] {
return from.map { self[$0] }
}
}
typealias Scale = [String]
struct ScaleGenerator {
let tonic: String
let mode: String
let pattern: String
let sharps: Scale = ["A", "A#", "B", "C", "C#", "D", "D#", "E", "F", "F#", "G", "G#"]
let flats: Scale = ["A", "Bb", "B", "C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab"]
init(tonic: String, scaleName: String, pattern: String = "mmmmmmmmmmm") {
self.tonic = tonic
self.mode = scaleName
self.pattern = pattern
}
var name: String {
return tonic.firstUppercased + " " + mode
}
var intervals: [Int] {
var index = 0
let intervals: [Character: Int] = ["m": 1, "M": 2, "A": 3]
return [0] + pattern.compactMap { intervals[$0] }
.map { (interval: Int) -> Int in
index += interval
return index
}.filter { $0 <= $0 % sharps.count }
}
var baseScale: Scale {
switch tonic {
case "F", "Bb", "Eb", "Ab", "Db", "Gb", "d", "g", "c", "bb", "eb":
return flats
default:
return sharps
}
}
func pitches() -> Scale {
baseScale.rotated(from: tonic.firstUppercased).pick(from: intervals)
}
}
|
At this point, I love how the pitches() method reads: “take the base scale, rotate it from the tonic note, then pick out the notes based on the scale’s intervals”.
The ScaleGenerator class itself isn’t too bad now: one method, and two calculated properties, each of which has some tests for it.
Other Thoughts
I find a lot of inspiration from other folks on exercism.io (example) who take a lot of care to think about their solution to make it clean and easy to follow. Much of the enjoyment I find in Swift is due in part to the design of the language and part to the many enthusiasts who are willing to share their work openly.
Last modified on 2020-09-04