Improved getRankedChords algorithm #43
Conversation
…at Five in all cases @aure
Co-authored-by: Aure <[email protected]>
Co-authored-by: Aure <[email protected]>
Co-authored-by: Aure <[email protected]>
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
| var isDouble: Bool { | ||
| switch self { | ||
| case .doubleFlat, .doubleSharp: return true | ||
| default: return false |
There was a problem hiding this comment.
Switch and Case Statement Alignment Violation: Case statements should vertically align with their enclosing switch statement. (switch_case_alignment)
| extension Accidental { | ||
| var isDouble: Bool { | ||
| switch self { | ||
| case .doubleFlat, .doubleSharp: return true |
There was a problem hiding this comment.
Switch and Case Statement Alignment Violation: Case statements should vertically align with their enclosing switch statement. (switch_case_alignment)
| !chord.bassNote.accidental.isDouble | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
| @@ -211,16 +262,32 @@ extension Chord { | |||
|
|
|||
| // prefer root notes not being uncommon enharmonics | |||
| returnArray.sort { ($0.root.canonicalNote.isUncommonEnharmonic ? 1 : 0) < ($1.root.canonicalNote.isUncommonEnharmonic ? 1 : 0) } | |||
|
|
|||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
| guard let searchNoteClass = rootNote.shiftUp(nextInterval)?.noteClass else { continue } | ||
| for noteArray in enharmonicNoteArrays where !usedNoteArrays.contains(noteArray) { | ||
| if noteArray.map({$0.noteClass}).contains(searchNoteClass) { | ||
| guard let matchedNote = noteArray.first(where: {$0.noteClass == searchNoteClass}) else { continue } |
There was a problem hiding this comment.
Line Length Violation: Line should be 120 characters or less: currently 131 characters (line_length)
| } | ||
| } | ||
| } | ||
| noteArray.sort { n1, n2 in |
There was a problem hiding this comment.
Identifier Name Violation: Variable name should be between 3 and 40 characters long: 'n1' (identifier_name)
Identifier Name Violation: Variable name should be between 3 and 40 characters long: 'n2' (identifier_name)
| /// - Parameters: | ||
| /// - pitchSet: Pitches to be analyzed | ||
| /// - allowTheoreticalChords: This algorithim will provide chords with double flats, double sharps, and inergonomic root notes like E# and Cb | ||
| public static func getRankedChords(from pitchSet: PitchSet, allowTheoreticalChords: Bool = false) -> [Chord] { |
There was a problem hiding this comment.
Cyclomatic Complexity Violation: Function should have complexity 10 or less: currently complexity equals 20 (cyclomatic_complexity)
Function Body Length Violation: Function body should span 40 lines or less excluding comments and whitespace: currently spans 57 lines (function_body_length)
| /// Get chords from a PitchSet, ranked by simplicity of notation | ||
| /// - Parameters: | ||
| /// - pitchSet: Pitches to be analyzed | ||
| /// - allowTheoreticalChords: This algorithim will provide chords with double flats, double sharps, and inergonomic root notes like E# and Cb |
There was a problem hiding this comment.
Line Length Violation: Line should be 120 characters or less: currently 147 characters (line_length)
| /// Get chords that match a set of pitches, ranking by least number of accidentals | ||
| public static func getRankedChords(from pitchSet: PitchSet) -> [Chord] { | ||
| var noteArrays: Set<[Note]> = [] | ||
|
|
There was a problem hiding this comment.
Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
| let accidentalCount = Accidental.allCases.count | ||
| let letterCount = Letter.allCases.count | ||
| let octaveCount = letterCount * accidentalCount | ||
|
|
There was a problem hiding this comment.
Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
| noteClass = NoteClass(letter, accidental: accidental) | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
| @@ -136,7 +142,7 @@ public struct Note: Equatable, Hashable, Codable { | |||
| let newLetterIndex = (noteClass.letter.rawValue + (shift.degree - 1)) | |||
| let newLetter = Letter(rawValue: newLetterIndex % Letter.allCases.count)! | |||
| let newMidiNoteNumber = Int(pitch.midiNoteNumber) + shift.semitones | |||
|
|
|||
There was a problem hiding this comment.
Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
| @@ -286,14 +308,14 @@ class ChordTests: XCTestCase { | |||
| let midiNotes: [Int8] = [54, 58, 61] | |||
| let fSharp = PitchSet(pitches: midiNotes.map { Pitch($0) } ) | |||
| let chords = Chord.getRankedChords(from: fSharp) | |||
| XCTAssertEqual(chords.map { $0.description }, ["G♭","F♯"]) | |||
| XCTAssertEqual(chords.map { $0.slashDescription }, ["G♭","F♯"]) | |||
There was a problem hiding this comment.
Comma Spacing Violation: There should be no space before and one after any comma. (comma)
| XCTAssertEqual(Chord(.C, type: .dominantSeventh).description, "C7") | ||
| let notes: [Int8] = [60, 67, 70, 76] | ||
| let pitchSet = PitchSet(pitches: notes.map { Pitch($0) } ) | ||
| let c7 = Chord.getRankedChords(from: pitchSet, allowTheoreticalChords: true) |
There was a problem hiding this comment.
Identifier Name Violation: Variable name should be between 3 and 40 characters long: 'c7' (identifier_name)
| func testTheortical() { | ||
| XCTAssertEqual(Chord(.C, type: .dominantSeventh).description, "C7") | ||
| let notes: [Int8] = [60, 67, 70, 76] | ||
| let pitchSet = PitchSet(pitches: notes.map { Pitch($0) } ) |
There was a problem hiding this comment.
Closing Brace Spacing Violation: Closing brace with closing parenthesis should not have any whitespaces in the middle. (closing_brace)
| XCTAssertEqual(c7.map { $0.description }, ["C7"]) | ||
| XCTAssertEqual(c7.map { $0.slashDescription }, ["C7"]) | ||
| } | ||
|
|
There was a problem hiding this comment.
Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
| @@ -2,6 +2,7 @@ import Tonic | |||
| import XCTest | |||
|
|
|||
| class ChordTests: XCTestCase { | |||
|
|
|||
There was a problem hiding this comment.
Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
| XCTAssertEqual(Key.Cb.noteSet.array.map { $0.noteClass.description }, | ||
| ["C♭", "D♭", "E♭", "F♭", "G♭", "A♭", "B♭"]) | ||
| XCTAssertEqual(Key.Cb.noteSet.array.sorted().map({ $0.noteClass.description }).sorted(), | ||
| ["A♭", "B♭","C♭", "D♭", "E♭", "F♭", "G♭"]) |
There was a problem hiding this comment.
Comma Spacing Violation: There should be no space before and one after any comma. (comma)
| case .dominantFlatFive: return "7b5" | ||
| case .dominantSharpFive: return "7#5" | ||
| case .dominantSeventhFlatFive: return "7b5" | ||
| case .dominantSeventhSharpFive: return "7#5" |
There was a problem hiding this comment.
Switch and Case Statement Alignment Violation: Case statements should vertically align with their enclosing switch statement. (switch_case_alignment)
| case .majorNinthSharpEleventh: return "^9#11" | ||
| case .dominantFlatFive: return "7b5" | ||
| case .dominantSharpFive: return "7#5" | ||
| case .dominantSeventhFlatFive: return "7b5" |
There was a problem hiding this comment.
Switch and Case Statement Alignment Violation: Case statements should vertically align with their enclosing switch statement. (switch_case_alignment)
| case .flatNinth: return "7b9" | ||
| case .sharpNinth: return "7#9" | ||
| case .dominantFlatNinth: return "7b9" | ||
| case .dominantSharpNinth: return "7#9" |
There was a problem hiding this comment.
Switch and Case Statement Alignment Violation: Case statements should vertically align with their enclosing switch statement. (switch_case_alignment)
| case .halfDiminishedNinth: return "Ø9" | ||
| case .dominantNinth: return "9" | ||
| case .dominantNinthSuspendedFourth: return "9sus4" | ||
| case .flatNinth: return "7b9" | ||
| case .sharpNinth: return "7#9" | ||
| case .dominantFlatNinth: return "7b9" |
There was a problem hiding this comment.
Switch and Case Statement Alignment Violation: Case statements should vertically align with their enclosing switch statement. (switch_case_alignment)
| @@ -315,11 +320,13 @@ extension ChordType: CustomStringConvertible { | |||
| case .majorSeventh: return "^7" | |||
| case .minorSeventh: return "m7" | |||
| case .minorMajorSeventh: return "m^7" | |||
| case .majorSeventhFlatFive: return "^7b5" | |||
| case .augmentedMajorSeventh: return "^7#5" | |||
There was a problem hiding this comment.
Switch and Case Statement Alignment Violation: Case statements should vertically align with their enclosing switch statement. (switch_case_alignment)
| case .halfDiminishedNinth: return "ø9" | ||
| case .dominantNinth: return "9" | ||
| case .dominantNinthSuspendedFourth: return "9sus4" | ||
| case .flatNinth: return "7♭9" | ||
| case .sharpNinth: return "7♯9" | ||
| case .dominantFlatNinth: return "7♭9" |
There was a problem hiding this comment.
Switch and Case Statement Alignment Violation: Case statements should vertically align with their enclosing switch statement. (switch_case_alignment)
| case .majorSeventhFlatFive: return "maj7(♭5)" | ||
| case .augmentedMajorSeventh: return "maj7(♯5)" | ||
| case .dominantSeventhFlatFive: return "7♭5" | ||
| case .dominantSeventhSharpFive: return "7♯5" |
There was a problem hiding this comment.
Switch and Case Statement Alignment Violation: Case statements should vertically align with their enclosing switch statement. (switch_case_alignment)
| @@ -253,27 +258,29 @@ extension ChordType: CustomStringConvertible { | |||
| case .majorSeventh: return "maj7" | |||
| case .minorSeventh: return "m7" | |||
| case .minorMajorSeventh: return "mMaj7" | |||
| case .majorSeventhFlatFive: return "maj7(♭5)" | |||
| case .augmentedMajorSeventh: return "maj7(♯5)" | |||
| case .dominantSeventhFlatFive: return "7♭5" | |||
There was a problem hiding this comment.
Switch and Case Statement Alignment Violation: Case statements should vertically align with their enclosing switch statement. (switch_case_alignment)
| @@ -253,27 +258,29 @@ extension ChordType: CustomStringConvertible { | |||
| case .majorSeventh: return "maj7" | |||
| case .minorSeventh: return "m7" | |||
| case .minorMajorSeventh: return "mMaj7" | |||
| case .majorSeventhFlatFive: return "maj7(♭5)" | |||
| case .augmentedMajorSeventh: return "maj7(♯5)" | |||
There was a problem hiding this comment.
Switch and Case Statement Alignment Violation: Case statements should vertically align with their enclosing switch statement. (switch_case_alignment)
| @@ -253,27 +258,29 @@ extension ChordType: CustomStringConvertible { | |||
| case .majorSeventh: return "maj7" | |||
| case .minorSeventh: return "m7" | |||
| case .minorMajorSeventh: return "mMaj7" | |||
| case .majorSeventhFlatFive: return "maj7(♭5)" | |||
There was a problem hiding this comment.
Switch and Case Statement Alignment Violation: Case statements should vertically align with their enclosing switch statement. (switch_case_alignment)
No description provided.