Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement letters for lab 03 of go-course #513

Closed
wants to merge 4 commits into from
Closed

Implement letters for lab 03 of go-course #513

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7b778f3
Implement letters for lab 03 of go-course
Jun 26, 2019
46c15e3
Merge branch 'master' into lab3
undrewb Jul 1, 2019
5a825a6
Remove comments from source code
Aug 15, 2019
15c4d76
Remove extra whitespace in test
Aug 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions 03_letters/undrewb/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package main

// https://github.com/anz-bank/go-course/tree/master/03_letters
undrewb marked this conversation as resolved.
Show resolved Hide resolved

import (
"fmt"
"io"
"os"
"sort"
"strings"
)

var out io.Writer = os.Stdout

func main() {
fmt.Fprintln(out, strings.Join(sortLetters(letters("aba")), "\n"))
}

func sortLetters(m map[rune]int) []string {
strs := make([]string, 0, len(m))
for r, c := range m {
strs = append(strs, fmt.Sprintf("%c:%d", r, c))
}
sort.Strings(strs)
return strs
}

func letters(s string) map[rune]int {
rc := make(map[rune]int)
for _, r := range s {
rc[r]++
}
return rc
}
143 changes: 143 additions & 0 deletions 03_letters/undrewb/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
package main

import (
"bytes"
"reflect"
"strconv"
"strings"
"testing"
)

// var out io.Writer = os.Stdout
undrewb marked this conversation as resolved.
Show resolved Hide resolved

func TestMainOutput(t *testing.T) {
var buf bytes.Buffer
out = &buf

main()

want := strconv.Quote("a:2\nb:1\n")
got := strconv.Quote(buf.String())

if got != want {
t.Errorf("actual: %s does not match expected: %s", got, want)
}
}

var sortLettersTestData = []struct {
name string
input string
want string
}{
{name: "lab example", input: "aba", want: "a:2,b:1"},
{name: "trailing newline", input: "aba\n", want: "\n:1,a:2,b:1"},
{name: "duplicate entries", input: "\t\t\\aba\n", want: "\t:2,\n:1,\\:1,a:2,b:1"},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that you are trying to achieve good test coverage, but I think this might be a little bit of overkill. It's important to consider what each test is trying to achieve when building out test cases. I feel that a lot of these cases are kind of testing the same thing (one repeated letter, random example, whitespace, duplicate entries, lab example) and many of these can be removed.

If you think about structuring your tests and having each test try to achieve / verify something in particular, you should be able to hit most of these cases with a couple of tests.

  • Empty input -> Corner case
  • Multiple letters (different languages if you want) -> Positive Case
  • Emoji input -> Verify data structure is using rune

{name: "one letter", input: "a", want: "a:1"},
{name: "one repeated letter", input: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", want: "b:61"},
{name: "repeated entry", input: "abababababababab", want: "a:8,b:8"},
{name: "empty string", input: "", want: ""},
{name: "random example", input: "blahflehblahbleh", want: "a:2,b:3,e:2,f:1,h:4,l:4"},
{name: "whitespace", input: "\t\t \t\t\t \t\n\n", want: "\t:6,\n:2, :4"},
{name: "Katakana",
input: " イロハニホヘト チリヌルヲ ワカヨタレソ ツネナラム ツネナラム ウヰノオクヤマ ケフコエテ アサキユメミシ ヱヒモセスン",
want: " :10,ア:1,イ:1,ウ:1,エ:1,オ:1,カ:1,キ:1,ク:1,ケ:1,コ:1,サ:1,シ:1,ス:1,セ:1,ソ:1,タ:1,チ:1,ツ:2,テ:1,ト:1,ナ:2," +
"ニ:1,ヌ:1,ネ:2,ノ:1,ハ:1,ヒ:1,フ:1,ヘ:1,ホ:1,マ:1,ミ:1,ム:2,メ:1,モ:1,ヤ:1,ユ:1,ヨ:1,ラ:2,リ:1,ル:1,レ:1,ロ:1,ワ:1,ヰ:1," +
"ヱ:1,ヲ:1,ン:1"},
{name: "Polish",
input: "Pchnąć w tę łódź jeża lub ośm skrzyń fig",
want: " :8,P:1,a:1,b:1,c:1,d:1,e:1,f:1,g:1,h:1,i:1,j:1,k:1,l:1,m:1,n:1,o:1,r:1,s:1,t:1,u:1,w:1,y:1,z:1,ó:1,ą:1,ć:1," +
"ę:1,ł:1,ń:1,ś:1,ź:1,ż:1"},
{name: "thai",
input: " ๏ เป็นมนุษย์สุดประเสริฐเลิศคุณค่า กว่าบรรดาฝูงสัตว์เดรัจฉาน " +
"จงฝ่าฟันพัฒนาวิชาการ อย่าล้างผลาญฤๅเข่นฆ่าบีฑาใคร " +
"ม่ถือโทษโกรธแช่งซัดฮึดฮัดด่า หัดอภัยเหมือนกีฬาอัชฌาสัย " +
"ปฏิบัติประพฤติกฎกำหนดใจ พูดจาให้จ๊ะๆ จ๋าๆ น่าฟังเอย ฯ",
want: "\t:1, :16,ก:6,ข:1,ค:3,ฆ:1,ง:5,จ:6,ฉ:1,ช:3,ซ:1,ฌ:1,ญ:1,ฎ:1,ฏ:1,ฐ:1,ฑ:1,ฒ:1,ณ:1,ด:10,ต:3,ถ:1," +
"ท:1,ธ:1,น:9,บ:3,ป:4,ผ:1,ฝ:2,พ:3,ฟ:2,ภ:1,ม:3,ย:5,ร:9,ฤ:2,ล:3,ว:3,ศ:1,ษ:2,ส:4,ห:4,ฬ:1,อ:6,ฮ:2,ฯ:1,ะ:3,ั" +
":12,า:19,ำ:1,ิ:6,ี:2,ึ:1,ื:2,ุ:3,ู:2,เ:7,แ:1,โ:2,ใ:3,ๅ:1,ๆ:2,็:1,่:10,้:2,๊:1,๋:1,์:2,๏:1"},
{name: "Russian",
input: "Съешь же ещё этих мягких французских булок да выпей чаю",
want: " :9,С:1,а:3,б:1,в:1,г:1,д:1,е:4,ж:1,з:1,и:3,й:1,к:3,л:1,м:1,н:1,о:1,п:1,р:1," +
"с:1,т:1,у:2,ф:1,х:3,ц:1,ч:1,ш:1,щ:1,ъ:1,ы:1,ь:1,э:1,ю:1,я:1,ё:1"},
{name: "Emojis",
input: "👾 🙇 💁 🙅 🙆 🙋 🙎 🐵 🙈 🙉 🙊 ❤️ 💔 💌 💕 💞 💓 💗 💖 💘 💝 💟 💜 💛 💚 💙",
want: " :25,❤:1,️:1,🐵:1,👾:1,💁:1,💌:1,💓:1,💔:1,💕:1,💖:1,💗:1,💘:1,💙:1,💚:1,💛:1,💜:1,💝:1,💞:1,💟:1," +
undrewb marked this conversation as resolved.
Show resolved Hide resolved
"🙅:1,🙆:1,🙇:1,🙈:1,🙉:1,🙊:1,🙋:1,🙎:1"},
}

func TestSortLetters(t *testing.T) {
for _, tt := range sortLettersTestData {
tt := tt // as per sean- suggestion on this discussion https://github.com/kyoh86/scopelint/issues/4
undrewb marked this conversation as resolved.
Show resolved Hide resolved
t.Run(tt.name, func(t *testing.T) {
got := strings.Join(sortLetters(letters(tt.input)), ",")
if got != tt.want {
t.Errorf("sortLetters() = \n%v,\nwant \n%v", got, tt.want)
}
})
}
}

var lettersTestData = []struct {
name string
input string
want map[rune]int
}{
{name: "lab example", input: "aba", want: map[rune]int{97: 2, 98: 1}},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please update the values for the want section of each case so you are using the actual char values rather than int values for the runes? Makes the test cases a lot easier to read.

{name: "lab example", input: "aba", want: map[rune]int{'a': 2, 'b': 1}},

{name: "trailing newline", input: "aba\n", want: map[rune]int{10: 1, 97: 2, 98: 1}},
{name: "duplicate entries", input: "\t\t\\aba\n", want: map[rune]int{9: 2, 10: 1, 92: 1, 97: 2, 98: 1}},
{name: "one letter", input: "a", want: map[rune]int{97: 1}},
{name: "one repeated letter",
input: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
want: map[rune]int{98: 61}},
{name: "repeated entry", input: "abababababababab", want: map[rune]int{97: 8, 98: 8}},
{name: "empty string", input: "", want: map[rune]int{}},
{name: "random example", input: "blahflehblahbleh", want: map[rune]int{97: 2, 98: 3, 101: 2, 102: 1, 104: 4, 108: 4}},
{name: "whitespace", input: "\t\t \t\t\t \t\n\n", want: map[rune]int{9: 6, 10: 2, 32: 4}},
{name: "Katakana",
input: " イロハニホヘト チリヌルヲ ワカヨタレソ ツネナラム ツネナラム ウヰノオクヤマ ケフコエテ アサキユメミシ ヱヒモセスン",
want: map[rune]int{32: 10, 12450: 1, 12452: 1, 12454: 1, 12456: 1, 12458: 1, 12459: 1, 12461: 1,
12463: 1, 12465: 1, 12467: 1, 12469: 1, 12471: 1, 12473: 1, 12475: 1, 12477: 1, 12479: 1, 12481: 1,
12484: 2, 12486: 1, 12488: 1, 12490: 2, 12491: 1, 12492: 1, 12493: 2, 12494: 1, 12495: 1, 12498: 1,
12501: 1, 12504: 1, 12507: 1, 12510: 1, 12511: 1, 12512: 2, 12513: 1, 12514: 1, 12516: 1, 12518: 1,
12520: 1, 12521: 2, 12522: 1, 12523: 1, 12524: 1, 12525: 1, 12527: 1, 12528: 1, 12529: 1, 12530: 1,
12531: 1}},
{name: "Polish",
input: "Pchnąć w tę łódź jeża lub ośm skrzyń fig",
want: map[rune]int{32: 8, 80: 1, 97: 1, 98: 1, 99: 1, 100: 1, 101: 1, 102: 1, 103: 1, 104: 1, 105: 1, 106: 1,
107: 1, 108: 1, 109: 1, 110: 1, 111: 1, 114: 1, 115: 1, 116: 1, 117: 1, 119: 1, 121: 1, 122: 1, 243: 1, 261: 1,
263: 1, 281: 1, 322: 1, 324: 1, 347: 1, 378: 1, 380: 1}},
{name: "thai",
input: " ๏ เป็นมนุษย์สุดประเสริฐเลิศคุณค่า กว่าบรรดาฝูงสัตว์เดรัจฉาน จงฝ่าฟันพัฒนาวิชาการ" +
" อย่าล้างผลาญฤๅเข่นฆ่าบีฑาใคร ม่ถือโทษโกรธแช่งซัดฮึดฮัดด่า" +
" หัดอภัยเหมือนกีฬาอัชฌาสัย ปฏิบัติประพฤติกฎกำหนดใจ พูดจาให้จ๊ะๆ จ๋าๆ น่าฟังเอย ฯ",
want: map[rune]int{9: 1, 32: 16, 3585: 6, 3586: 1, 3588: 3, 3590: 1, 3591: 5, 3592: 6, 3593: 1, 3594: 3,
3595: 1, 3596: 1, 3597: 1, 3598: 1, 3599: 1, 3600: 1, 3601: 1, 3602: 1, 3603: 1, 3604: 10, 3605: 3,
3606: 1, 3607: 1, 3608: 1, 3609: 9, 3610: 3, 3611: 4, 3612: 1, 3613: 2, 3614: 3, 3615: 2, 3616: 1, 3617: 3,
3618: 5, 3619: 9, 3620: 2, 3621: 3, 3623: 3, 3624: 1, 3625: 2, 3626: 4, 3627: 4, 3628: 1, 3629: 6, 3630: 2,
3631: 1, 3632: 3, 3633: 12, 3634: 19, 3635: 1, 3636: 6, 3637: 2, 3638: 1, 3639: 2, 3640: 3, 3641: 2, 3648: 7,
3649: 1, 3650: 2, 3651: 3, 3653: 1, 3654: 2, 3655: 1, 3656: 10, 3657: 2, 3658: 1, 3659: 1, 3660: 2, 3663: 1}},
{name: "Russian",
input: "Съешь же ещё этих мягких французских булок да выпей чаю",
want: map[rune]int{32: 9, 1057: 1, 1072: 3, 1073: 1, 1074: 1, 1075: 1, 1076: 1, 1077: 4, 1078: 1, 1079: 1, 1080: 3,
1081: 1, 1082: 3, 1083: 1, 1084: 1, 1085: 1, 1086: 1, 1087: 1, 1088: 1, 1089: 1, 1090: 1, 1091: 2, 1092: 1, 1093: 3,
1094: 1, 1095: 1, 1096: 1, 1097: 1, 1098: 1, 1099: 1, 1100: 1, 1101: 1, 1102: 1, 1103: 1, 1105: 1}},
{name: "Emojis",
input: "👾 🙇 💁 🙅 🙆 🙋 🙎 🐵 🙈 🙉 🙊 ❤️" +
" 💔 💌 💕 💞 💓 💗 💖 💘 💝 💟 💜 💛 💚 💙",
want: map[rune]int{32: 25, 10084: 1, 65039: 1, 128053: 1, 128126: 1, 128129: 1, 128140: 1,
128147: 1, 128148: 1, 128149: 1, 128150: 1, 128151: 1, 128152: 1, 128153: 1, 128154: 1,
128155: 1, 128156: 1, 128157: 1, 128158: 1, 128159: 1, 128581: 1, 128582: 1, 128583: 1,
128584: 1, 128585: 1, 128586: 1, 128587: 1, 128590: 1}},
}

func TestLetters(t *testing.T) {
for _, tt := range lettersTestData {
tt := tt // as per sean- suggestion on this discussion https://github.com/kyoh86/scopelint/issues/4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might have been suggested already, but I don't think this comment is required. Can you please remove this?

t.Run(tt.name, func(t *testing.T) {
got := letters(tt.input)
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("sortLetters() = \n%v,\nwant \n%v", got, tt.want)
}
})
}
}