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.

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

[WIP] say: update tests #867

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 9 additions & 18 deletions exercises/say/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ Handle the basic case of 0 through 99.
If the input to the program is `22`, then the output should be
`'twenty-two'`.

Your program should complain loudly if given a number outside the
blessed range.
Your program should throw an error if given a number outside the
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can stay as it is.

range of 0 through 99.

Some good test cases for this program are:

- 0
- 14
- 50
- 98
- -1
- 100
- 99
- 100 (currently an error case)
Copy link
Contributor

Choose a reason for hiding this comment

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

With changing the function to use an unsigned int64 I don't think we need to stray from the description in the problem-specifications repo.


### Extension

Expand All @@ -28,39 +28,30 @@ loud.

## Step 2

Handle the the case of 0 through 999,999,999,999.

Implement breaking a number up into chunks of thousands.

So `1234567890` should yield a list like 1, 234, 567, and 890, while the
far simpler `1000` should yield just 1 and 0.

The program must also report any values that are out of range.
The program must also report any values that are larger than 999,999,999,999.

## Step 3

Now handle inserting the appropriate scale word between those chunks.

So `1234567890` should yield `'1 billion 234 million 567 thousand 890'`

The program must also report any values that are out of range. It's
fine to stop at "trillion".
The program must also report any values that are larger than 999,999,999,999.

## Step 4

Put it all together to get nothing but plain English.

`12345` should give `twelve thousand three hundred forty-five`.

The program must also report any values that are out of range.

### Extensions

Use _and_ (correctly) when spelling out the number in English:

- 14 becomes "fourteen".
- 100 becomes "one hundred".
- 120 becomes "one hundred and twenty".
- 1002 becomes "one thousand and two".
- 1323 becomes "one thousand three hundred and twenty-three".
The program must also report any values that are larger than 999,999,999,999.

## Running the tests

Expand Down
28 changes: 18 additions & 10 deletions exercises/say/example.go
Original file line number Diff line number Diff line change
@@ -1,45 +1,53 @@
package say

import "errors"

const testVersion = 1

var small = []string{"zero", "one", "two", "three", "four", "five", "six",
"seven", "eight", "nine", "ten", "eleven", "twelve", "thirteen",
"fourteen", "fifteen", "sixteen", "seventeen", "eighteen", "nineteen"}
var tens = []string{"ones", "ten", "twenty", "thirty", "forty",
"fifty", "sixty", "seventy", "eighty", "ninety"}
var scale = []string{"thousand", "million", "billion",
"trillion", "quadrillion", "quintillion"}
var scale = []string{"thousand", "million", "billion"}

func Say(n uint64) string {
func Say(n uint64) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be changed to a signed int64, so we can validate negative numbers also.

I like the change in the function signature to include an error. As we can't return -1 when n is out of range this is the best way to go. 👍

if n >= 1000000000000 {
return "", errors.New("value out of range")
}
switch {
case n < 20:
return small[n]
return small[n], nil
case n < 100:
t := tens[n/10]
s := n % 10
if s > 0 {
t += "-" + small[s]
}
return t
return t, nil
case n < 1000:
h := small[n/100] + " hundred"
s := n % 100
if s > 0 {
h += " " + Say(s)
temp, _ := Say(s)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this variable name, I understand it's to signify temporary, but I think x would suffice, the use of temp rather than a letter seems to infer more importance on it than is necessary.

I also think this error should be handled. At this point in the function it shouldn't error out, but still it's better to handle it than ignore it I think.

h += " " + temp
}
return h
return h, nil
}
sx := ""
if p := n % 1000; p > 0 {
sx = Say(p)
sx, _ = Say(p)
}
for i := 0; n >= 1000; i++ {
n /= 1000
if p := n % 1000; p > 0 {
ix := Say(p) + " " + scale[i]
temp, _ := Say(p)
ix := temp + " " + scale[i]
if sx > "" {
ix += " " + sx
}
sx = ix
}
}
return sx
return sx, nil
}
15 changes: 14 additions & 1 deletion exercises/say/say_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ var tests = []struct {
"three hundred twenty-one thousand " +
"one hundred twenty-three"},
{0, "zero"},
}
var errorTests = []struct {
uint64
string
}{
{math.MaxUint64, "eighteen quintillion " +
"four hundred forty-six quadrillion " +
"seven hundred forty-four trillion " +
Expand All @@ -40,8 +45,16 @@ var tests = []struct {

func TestSay(t *testing.T) {
for _, test := range tests {
if s := Say(test.uint64); s != test.string {
if s, _ := Say(test.uint64); s != test.string {
t.Errorf("Say(%d) = %q. Want %q.", test.uint64, s, test.string)
}
}
}

func TestErrorSay(t *testing.T) {
for _, test := range errorTests {
if _, e := Say(test.uint64); e == nil {
t.Errorf("Say(%d) should throw an error", test.uint64)
}
}
}