From 8e13df76d8c175c7a214f031cbbd796dcc052bba Mon Sep 17 00:00:00 2001 From: Tim Vermeulen Date: Sun, 16 Oct 2016 15:57:23 +0200 Subject: [PATCH 1/7] Fixed bug when people ask for "top x" with negative x --- Sources/Camille/KarmaService.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Camille/KarmaService.swift b/Sources/Camille/KarmaService.swift index e0a9982..7b4049f 100644 --- a/Sources/Camille/KarmaService.swift +++ b/Sources/Camille/KarmaService.swift @@ -185,8 +185,8 @@ final class KarmaService: SlackMessageService { return "<@\(bot.me.id)> top".lowercased() } private func topKarma(maxList: Int, in storage: Storage) -> String { - guard maxList != 0 else { - return "Top 0? You must work in QA." + guard maxList > 0 else { + return "Top \(maxList)? You must work in QA." } func karmaForUser(_ user: String) -> Int { From c34d8455338b54dcb2d645d5a000ec3ba74ab8ad Mon Sep 17 00:00:00 2001 From: Tim Vermeulen Date: Sun, 16 Oct 2016 15:58:37 +0200 Subject: [PATCH 2/7] Remove unnecessary braces --- Sources/Camille/KarmaService.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Camille/KarmaService.swift b/Sources/Camille/KarmaService.swift index 7b4049f..e9e2ab2 100644 --- a/Sources/Camille/KarmaService.swift +++ b/Sources/Camille/KarmaService.swift @@ -199,10 +199,10 @@ final class KarmaService: SlackMessageService { let responsePrefix: String let numberToShow: Int - if (maxList > 20) { + if maxList > 20 { numberToShow = maxList > sortedUsersAndKarma.count ? sortedUsersAndKarma.count : 20 responsePrefix = "Yeah, that's too many. Here's the top \(numberToShow):" - } else if (maxList > sortedUsersAndKarma.count) { + } else if maxList > sortedUsersAndKarma.count { numberToShow = sortedUsersAndKarma.count responsePrefix = "We only have \(numberToShow):" } else { From 0bd31e314d66d4ac7078811ce8f81bf97d5a76a7 Mon Sep 17 00:00:00 2001 From: Tim Vermeulen Date: Sun, 16 Oct 2016 16:00:41 +0200 Subject: [PATCH 3/7] Improve variable/function names --- Sources/Camille/KarmaService.swift | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/Sources/Camille/KarmaService.swift b/Sources/Camille/KarmaService.swift index e9e2ab2..bb76eca 100644 --- a/Sources/Camille/KarmaService.swift +++ b/Sources/Camille/KarmaService.swift @@ -189,29 +189,28 @@ final class KarmaService: SlackMessageService { return "Top \(maxList)? You must work in QA." } - func karmaForUser(_ user: String) -> Int { + func karma(for user: String) -> Int { return storage.get(Int.self, in: .in("Karma"), key: user, or: 0) } let users = storage.allKeys(.in("Karma")) - let sortedUsersAndKarma = users - .map { ($0, karmaForUser($0)) } - .sorted(by: { $0.1 > $1.1 }) + .map { (name: $0, karma: karma(for: $0)) } + .sorted(by: { $0.karma > $1.karma }) let responsePrefix: String let numberToShow: Int if maxList > 20 { - numberToShow = maxList > sortedUsersAndKarma.count ? sortedUsersAndKarma.count : 20 + numberToShow = maxList > users.count ? users.count : 20 responsePrefix = "Yeah, that's too many. Here's the top \(numberToShow):" - } else if maxList > sortedUsersAndKarma.count { - numberToShow = sortedUsersAndKarma.count + } else if maxList > users.count { + numberToShow = users.count responsePrefix = "We only have \(numberToShow):" } else { numberToShow = maxList responsePrefix = "Top \(numberToShow):" } - return sortedUsersAndKarma.prefix(numberToShow).reduce(responsePrefix, { (partialResponse, userAndKarma) in - partialResponse + "\n<@\(userAndKarma.0)>: \(userAndKarma.1)" + return users.prefix(numberToShow).reduce(responsePrefix, { partialResponse, user in + partialResponse + "\n<@\(user.name)>: \(user.karma)" }) } } From a136526177dd8d7b26be7f197bb1d01c96884f9a Mon Sep 17 00:00:00 2001 From: Tim Vermeulen Date: Sun, 16 Oct 2016 16:04:50 +0200 Subject: [PATCH 4/7] Simplify return string generation --- Sources/Camille/KarmaService.swift | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/Sources/Camille/KarmaService.swift b/Sources/Camille/KarmaService.swift index bb76eca..653fa14 100644 --- a/Sources/Camille/KarmaService.swift +++ b/Sources/Camille/KarmaService.swift @@ -200,17 +200,20 @@ final class KarmaService: SlackMessageService { let numberToShow: Int if maxList > 20 { numberToShow = maxList > users.count ? users.count : 20 - responsePrefix = "Yeah, that's too many. Here's the top \(numberToShow):" + responsePrefix = "Yeah, that's too many. Here's the top" } else if maxList > users.count { numberToShow = users.count - responsePrefix = "We only have \(numberToShow):" + responsePrefix = "We only have" } else { numberToShow = maxList - responsePrefix = "Top \(numberToShow):" + responsePrefix = "Top" } - return users.prefix(numberToShow).reduce(responsePrefix, { partialResponse, user in - partialResponse + "\n<@\(user.name)>: \(user.karma)" - }) + let list = users + .prefix(numberToShow) + .map { user in "<@\(user.name)>: \(user.karma)" } + .joined(separator: "\n") + + return "\(responsePrefix) \(numberToShow):\n\(list)" } } From 973af663cfebb971505f919a5e68915b4b5d74ad Mon Sep 17 00:00:00 2001 From: Tim Vermeulen Date: Sun, 16 Oct 2016 16:08:17 +0200 Subject: [PATCH 5/7] Fix bug where the length of the given list could be over 20 --- Sources/Camille/KarmaService.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Camille/KarmaService.swift b/Sources/Camille/KarmaService.swift index 653fa14..a25155c 100644 --- a/Sources/Camille/KarmaService.swift +++ b/Sources/Camille/KarmaService.swift @@ -199,7 +199,7 @@ final class KarmaService: SlackMessageService { let responsePrefix: String let numberToShow: Int if maxList > 20 { - numberToShow = maxList > users.count ? users.count : 20 + numberToShow = max(users.count, 20) responsePrefix = "Yeah, that's too many. Here's the top" } else if maxList > users.count { numberToShow = users.count From f0095b25a8d2623eaf477963b12eaa0c18119d40 Mon Sep 17 00:00:00 2001 From: Tim Vermeulen Date: Sun, 16 Oct 2016 16:09:57 +0200 Subject: [PATCH 6/7] Improve spacing --- Sources/Camille/KarmaService.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/Camille/KarmaService.swift b/Sources/Camille/KarmaService.swift index a25155c..a699618 100644 --- a/Sources/Camille/KarmaService.swift +++ b/Sources/Camille/KarmaService.swift @@ -185,19 +185,19 @@ final class KarmaService: SlackMessageService { return "<@\(bot.me.id)> top".lowercased() } private func topKarma(maxList: Int, in storage: Storage) -> String { - guard maxList > 0 else { - return "Top \(maxList)? You must work in QA." - } + guard maxList > 0 else { return "Top \(maxList)? You must work in QA." } func karma(for user: String) -> Int { return storage.get(Int.self, in: .in("Karma"), key: user, or: 0) } + let users = storage.allKeys(.in("Karma")) .map { (name: $0, karma: karma(for: $0)) } .sorted(by: { $0.karma > $1.karma }) let responsePrefix: String let numberToShow: Int + if maxList > 20 { numberToShow = max(users.count, 20) responsePrefix = "Yeah, that's too many. Here's the top" From ed6ebf767bba2df19c71bec91cf010b70aa033ce Mon Sep 17 00:00:00 2001 From: Tim Vermeulen Date: Sun, 16 Oct 2016 16:17:53 +0200 Subject: [PATCH 7/7] Simplify storage request --- Sources/Camille/KarmaService.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Camille/KarmaService.swift b/Sources/Camille/KarmaService.swift index a699618..a57f995 100644 --- a/Sources/Camille/KarmaService.swift +++ b/Sources/Camille/KarmaService.swift @@ -188,7 +188,7 @@ final class KarmaService: SlackMessageService { guard maxList > 0 else { return "Top \(maxList)? You must work in QA." } func karma(for user: String) -> Int { - return storage.get(Int.self, in: .in("Karma"), key: user, or: 0) + return storage.get(in: .in("Karma"), key: user, or: 0) } let users = storage.allKeys(.in("Karma"))