Removing duplicates after tokenization

[tzdybal]

This is PR for issue #1183.

---

This change is 

[ashwin95r]

---

Reviewed 6 of 6 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

x/x.go, line 198 at r1 (raw file):

// removes duplicates from a sorted slice of strings. Changes underylying array.
func RemoveDuplicates(s []string) (out []string) {

We could move the sort to this function.

---

Comments from Reviewable

[manishrjain]

Address comments before submitting.

---

Reviewed 2 of 6 files at r1, 5 of 5 files at r2.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

tok/tok.go, line 337 at r2 (raw file):

			tokens[i] = encodeToken(trigram, t.Identifier())
		}
		x.RemoveDuplicates(tokens)

tokens = x.RemoveDuplicates(tokens) ?? Just like you've done above: terms = x.RemoveDuplicates(terms)

---

tok/tok_test.go, line 204 at r2 (raw file):

	require.NoError(t, err)

	expected := []string{" chief", " weapon", " surpris", " fear", " ruthless", " effici"}

Are we actually adding spaces in our tokens?

---

x/x.go, line 198 at r1 (raw file):

Previously, ashwin95r (Ashwin Ramesh) wrote…

We could move the sort to this function.

So, out is using the same array as s. And that causes confusion in the usage (as witnessed above). Just don't return out in that case. At least it would be clear that it's modifying the passed slice.

---

Comments from Reviewable

[tzdybal]

Review status: 4 of 7 files reviewed at latest revision, 3 unresolved discussions.

---

tok/tok.go, line 337 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

tokens = x.RemoveDuplicates(tokens) ?? Just like you've done above: terms = x.RemoveDuplicates(terms)

Done.

---

tok/tok_test.go, line 204 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Are we actually adding spaces in our tokens?

No, we're using per-tokenizer prefix. For the simplicity of test I used space instead of real tokenizer-identifier (it's much easier to type expected values that way).

---

x/x.go, line 198 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

So, out is using the same array as s. And that causes confusion in the usage (as witnessed above). Just don't return out in that case. At least it would be clear that it's modifying the passed slice.

Actually we have to return new slice, because the slice size may change. Added comment about the usage of this function.

---

Comments from Reviewable

[tzdybal]

Merged to master (8e412fd).