Repository: remotemobprogramming/mob
Pull request #187 (2021-07-24 15:55:32)

Improve next mobber suggestion
The existing heuristic which suggests the next mobber is relatively simple. The PR tries to add some improvements, namely:

* Do not suggest the current Git user if there are other mobbers in the list.
* Do not list the name of a previous committer multiple times in a row.
* Apply a simple lookahead in order to also suggest persons which might have been absent in the previous mob round.

Hopefully the tests make my intensions clear.

simonharrer: Looks great. Thanks for making this simple algorithm better.

Can you please use the wording next typist or next driver instead of next mobber? And we need a changelog entry in the CHANGELOG.md.

I think we can ship this with the next minor release. 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index dcee125..4525545 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,8 @@
+# 1.9.0
+- `mob next` does not show the same committer multiple times in the list of previous committers.
+- `mob next` does not suggest the current Git user to be the next typist as long as there were other persons involved in the mob.
+- `mob next` performs a simple lookahead to also suggest persons who might have been absent only during the last mob round.
+
 # 1.8.0
 - When user.name is not set in the git config, mob no longer shows an error but a warning with a help how to fix it.
 
diff --git a/find_next.go b/find_next.go
new file mode 100644
index 0000000..534297e
--- /dev/null
+++ b/find_next.go
@@ -0,0 +1,56 @@
+package main
+
+func findNextTypist(lastCommitters []string, gitUserName string) (nextTypist string, previousCommitters []string) {
+	numberOfLastCommitters := len(lastCommitters)
+	for i := 0; i < numberOfLastCommitters; i++ {
+		if lastCommitters[i] == gitUserName && i > 0 {
+			nextTypist = lastCommitters[i-1]
+			if nextTypist != gitUserName {
+				// '2*i+1' defines how far we look ahead. It is the number of already processed elements.
+				lookaheadThreshold := min(2*i+1, len(lastCommitters))
+				previousMobber := lookahead(lastCommitters[:i], lastCommitters[i:lookaheadThreshold])
+				if previousMobber != "" {
+					nextTypist = previousMobber
+				}
+				return
+			}
+		}
+		// Do not add the last committer multiple times.
+		if i == 0 || previousCommitters[0] != lastCommitters[i] {
+			previousCommitters = prepend(previousCommitters, lastCommitters[i])
+		}
+	}
+	return
+}
+
+func lookahead(processedCommitters []string, previousCommitters []string) (nextTypist string) {
+	for i := 0; i < len(previousCommitters); i++ {
+		if !contains(processedCommitters, previousCommitters[i]) {
+			nextTypist = previousCommitters[i]
+		}
+	}
+	return
+}
+
+func contains(list []string, element string) bool {
+	for i := 0; i < len(list); i++ {
+		if list[i] == element {
+			return true
+		}
+	}
+	return false
+}
+
+func min(a int, b int) int {
+	if a < b {
+		return a
+	}
+	return b
+}
+
+func prepend(list []string, element string) []string {
+	list = append(list, element)
+	copy(list[1:], list)
+	list[0] = element
+	return list
+}
diff --git a/mob.go b/mob.go
index 694aa52..3513e24 100644
--- a/mob.go
+++ b/mob.go
@@ -934,17 +934,10 @@ func showNext(configuration Configuration) {
 	if numberOfLines < 1 {
 		return
 	}
-	var history = ""
-	for i := 0; i < len(lines); i++ {
-		if lines[i] == gitUserName && i > 0 {
-			sayInfo("Committers after your last commit: " + history)
-			sayInfo("***" + lines[i-1] + "*** is (probably) next.")
-			return
-		}
-		if history != "" {
-			history = ", " + history
-		}
-		history = lines[i] + history
+	nextTypist, previousCommitters := findNextTypist(lines, gitUserName)
+	if nextTypist != "" {
+		sayInfo("Committers after your last commit: " + strings.Join(previousCommitters, ", "))
+		sayInfo("***" + nextTypist + "*** is (probably) next.")
 	}
 }
 


Test patch:
diff --git a/find_next_test.go b/find_next_test.go
new file mode 100644
index 0000000..429465c
--- /dev/null
+++ b/find_next_test.go
@@ -0,0 +1,86 @@
+package main
+
+import (
+	"testing"
+)
+
+func TestFindNextTypistNoCommits(t *testing.T) {
+	lastCommitters := []string{}
+
+	nextTypist, history := findNextTypist(lastCommitters, "alice")
+
+	equals(t, nextTypist, "")
+	equals(t, history, []string(nil))
+}
+
+func TestFindNextTypistOnlyCurrentCommitterInList(t *testing.T) {
+	lastCommitters := []string{"alice", "alice", "alice"}
+
+	nextTypist, history := findNextTypist(lastCommitters, "alice")
+
+	equals(t, nextTypist, "alice")
+	equals(t, history, []string{"alice"})
+}
+
+func TestFindNextTypistCurrentCommitterAlternatingWithOneOtherPerson(t *testing.T) {
+	lastCommitters := []string{"alice", "bob", "alice", "bob", "alice"}
+
+	nextTypist, history := findNextTypist(lastCommitters, "alice")
+
+	equals(t, nextTypist, "bob")
+	equals(t, history, []string{"bob", "alice"})
+}
+
+func TestFindNextTypistCurrentCommitterCommittedBefore(t *testing.T) {
+	lastCommitters := []string{"alice", "alice", "bob", "alice"}
+
+	nextTypist, history := findNextTypist(lastCommitters, "alice")
+
+	equals(t, nextTypist, "bob")
+	equals(t, history, []string{"bob", "alice"})
+}
+
+func TestFindNextTypistThreeCommitters(t *testing.T) {
+	lastCommitters := []string{"alice", "bob", "craig", "alice"}
+
+	nextTypist, history := findNextTypist(lastCommitters, "alice")
+
+	equals(t, nextTypist, "craig")
+	equals(t, history, []string{"craig", "bob", "alice"})
+}
+
+func TestFindNextTypistIgnoreMultipleCommitsFromSamePerson(t *testing.T) {
+	lastCommitters := []string{"alice", "bob", "craig", "craig", "alice"}
+
+	nextTypist, history := findNextTypist(lastCommitters, "alice")
+
+	equals(t, nextTypist, "craig")
+	equals(t, history, []string{"craig", "bob", "alice"})
+}
+
+func TestFindNextTypistSuggestCommitterBeforeLastCommit(t *testing.T) {
+	lastCommitters := []string{"alice", "bob", "craig", "alice", "bob", "dan"}
+
+	nextTypist, history := findNextTypist(lastCommitters, "alice")
+
+	equals(t, nextTypist, "dan")
+	equals(t, history, []string{"craig", "bob", "alice"})
+}
+
+func TestFindNextTypistSuggestCommitterBeforeLastCommitInThreshold(t *testing.T) {
+	lastCommitters := []string{"alice", "bob", "craig", "alice", "bob", "dan", "erik", "fin"}
+
+	nextTypist, history := findNextTypist(lastCommitters, "alice")
+
+	equals(t, nextTypist, "erik")
+	equals(t, history, []string{"craig", "bob", "alice"})
+}
+
+func TestFindNextTypistIgnoreCommitterBeforeLastCommitOutsideThreshold(t *testing.T) {
+	lastCommitters := []string{"alice", "bob", "craig", "alice", "craig", "bob", "alice", "fin"}
+
+	nextTypist, history := findNextTypist(lastCommitters, "alice")
+
+	equals(t, nextTypist, "craig")
+	equals(t, history, []string{"craig", "bob", "alice"})
+}