Learning Go and looking for advice and feedback

Hi,

I have the code below I am using to learn coding in Go.
It is a bowling game, it will calculate the final score and show visual representation of the game.

I am looking for advice and feedback about this code, so I can check if I am learning correct and if possible learn new things from your comment. Any feedback will be much appreciated.

The code is run-able on playground also: https://play.golang.org/p/QHf5Cqmx8M

package main

import (
	"fmt"
	"strconv"
)

const (
	maxPin    int = 10
	maxFrames int = 10
)

type frameValue interface {
	Value() int
	FirstBallValue() int
	SecondBallValue() int
	GetNext() frameValue
	SetNext(frameValue)
}

type frame struct {
	FirstBall  int
	SecondBall int

	next frameValue
}

type extraBallFrame struct {
	*frame
	ExtraBall int
}

// Game : Bownling game
type Game struct {
	head frameValue
	tail frameValue
}

// ### frame
func (f *frame) IsStrike() bool {
	return f.FirstBall == maxPin
}

func (f *frame) IsSpare() bool {
	if f.IsStrike() {
		return false
	}
	return (f.FirstBall + f.SecondBall) == maxPin
}

func (f *frame) String() string {
	if f.IsStrike() {
		return "X"
	}
	if f.IsSpare() {
		return fmt.Sprintf("%v/", f.FirstBall)
	}
	return fmt.Sprintf("%v%v", f.FirstBall, f.SecondBall)
}

func (f *frame) Value() int {
	sum := f.FirstBall + f.SecondBall
	if f.next != nil {
		if f.IsSpare() {
			sum += f.next.FirstBallValue()
		}
		if f.IsStrike() {
			sum += f.next.FirstBallValue() + f.next.SecondBallValue()
		}
	}
	return sum
}

func (f *frame) FirstBallValue() int {
	return f.FirstBall
}

func (f *frame) SecondBallValue() int {
	if f.IsStrike() && f.next != nil {
		return f.next.FirstBallValue()
	}
	return f.SecondBall
}
func (f *frame) SetNext(v frameValue) {
	f.next = v
}

func (f *frame) GetNext() frameValue {
	return f.next
}

// ### extraBallFrame
func (e *extraBallFrame) Value() int {
	return e.frame.Value()
}
func (e *extraBallFrame) FirstBallValue() int {
	return e.frame.FirstBallValue()
}
func (e *extraBallFrame) SecondBallValue() int {
	return e.SecondBall + e.ExtraBall
}
func (e *extraBallFrame) String() string {
	s := ""

	if e.FirstBall == maxPin {
		s += "X"
	} else {
		s += strconv.Itoa(e.FirstBall)
	}
	
	if e.SecondBall == maxPin {
		s += "X"
	} else if (e.FirstBall + e.SecondBall) == maxPin {
		s += "/"
	} else {
		s += strconv.Itoa(e.SecondBall)
	}

	if e.ExtraBall == maxPin {
		s += "X"
	} else {
		s += strconv.Itoa(e.ExtraBall)
	}
	return s
}


// #### Game
func (g *Game) add(frame frameValue) {
	if g.head == nil {
		g.head = frame
		g.tail = g.head
	} else {
		g.tail.SetNext(frame)
		g.tail = frame
	}
}

func (g *Game) addFrameInternal(ball1, ball2 int) {
	g.add(&frame{ball1, ball2, nil})
}

func (g *Game) addFrameExtraInternal(ball1, ball2, extraBall int) {
	g.add(&extraBallFrame{&frame{ball1, ball2, nil}, extraBall})
}

// AddFrame : Add frame with value from two ball's
func (g *Game) AddFrame(ball1, ball2 int) *Game {
	g.addFrameInternal(ball1, ball2)
	return g
}

// AddFrameExtra : Add frame with value from three ball's. That should be the latest frame
func (g *Game) AddFrameExtra(ball1, ball2, extraBall int) *Game {
 	g.addFrameExtraInternal(ball1, ball2, extraBall)
	return g
}

// AddStrike : Add frame as strike
func (g *Game) AddStrike() *Game {
	g.addFrameInternal(10, 0)
	return g
}

// AddSpare : Add rame as spare, second ball value is calculated
func (g *Game) AddSpare(ball1 int) *Game {
	g.addFrameInternal(ball1, (maxPin - ball1))
	return g
}

// AddMiss : Add frame as missing both ball's
func (g *Game) AddMiss() *Game {
	g.addFrameInternal(0, 0)
	return g
}

// CalculateScore : 
func (g *Game) CalculateScore() int {
	var score int
	for e := g.head; e != nil; e = e.GetNext() {
		score += e.Value()
	}
	return score
}
// Show : 
func (g *Game) Show() string {
	var result string
	for e := g.head; e != nil; e = e.GetNext() {
		result += fmt.Sprintf("%v,", e)
	}
	return result[:len(result)-1]
}

func testGame(game *Game, shouldBeValue int, shouldBeString string)  {
	asIsValue := game.CalculateScore()
	if shouldBeValue != asIsValue {
		fmt.Println("ERROR - Expected:", shouldBeValue, "Result:", asIsValue)
	}

	asIsString := game.Show()
	if shouldBeString != asIsString {
		fmt.Println("ERROR - Expected:", shouldBeString, "Result:", asIsString)
	}
}

func main()  {
	var shouldBeValue int
	var shouldBeString string
	var game *Game

	shouldBeValue = 270
	shouldBeString = "X,X,X,X,X,X,X,X,X,9/1"
	game = &Game{}
	game.AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddFrameExtra(9, 1, 1)
	testGame(game, shouldBeValue, shouldBeString)

	shouldBeValue = 271
	shouldBeString = "X,X,X,X,X,X,X,X,X,1/X"
	game = &Game{}
	game.AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddFrameExtra(1, 9, 10)
	testGame(game, shouldBeValue, shouldBeString)

	shouldBeValue = 300
	shouldBeString = "X,X,X,X,X,X,X,X,X,XXX"
	game = &Game{}
	game.AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddFrameExtra(10, 10, 10)
	testGame(game, shouldBeValue, shouldBeString)

	shouldBeValue = 20
	shouldBeString = "11,11,11,11,11,11,11,11,11,11"
	game = &Game{}
	game.AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1)
	testGame(game, shouldBeValue, shouldBeString)

	fmt.Println("Finished")
}

If I run the play, it just prints

Finished

What is supposed to happen?

I looks like you’ve build your own unit test logic. Take a look at the existing testing package which should be used for unit tests.

That is the normal expectation, it will just print more stuff if the code “fail”.
Try to change some value and you will see it.

But the code is working fine, it works as expected.
If you can, please check the code regarding Go way of do stuff and solve problem. Like I am using composition and method override, give me some feedback if you believe it could be better or solved in another way.

I am using testing package locally on my computer, but I put in this way here because it can be run-able in playgound.

Thanks you so much for spending time on it.

For example, I don’t like functions below.
Both does nothing, but I am “forced” to implement it.
In a normal OO I just need to override function func (e *extraBallFrame) SecondBallValue() int, but here I had to expose and implement Value() and FirstBallValue() also.

Do I have another option?

func (e *extraBallFrame) Value() int {
	return e.frame.Value()
}
func (e *extraBallFrame) FirstBallValue() int {
	return e.frame.FirstBallValue()
}

I don’t quite understand why you define the interface frameValue if you only implement it completely for frame. Couldn’t you just drop the interface and use methods on frame?

What is the logic behind frame and extraBallFrame?

Forgive me, but I know nothing about bowling and it is hard to understand the logic just by reading your code.

Bowling game contains 10 frames, on each frame you can roll 2 ball’s.
So I have frame struct with fields FirstBall and SecondBall.
But the last frame can be one with 2 ball’s or 3 ball’s, depending how game is going on.

So I have extraBallFrame struct that “inherits” from frame struct, just to have this extra ball.

On extraBallFrame I just need SecondBallValue(), but as it is called from method Value(), if I do not implement it on extraBallFrame, otherwise Value() from frame will just use method SecondBallValue() from frame, not from extraBallFrame.

Then I don’t understand why you think you are ‘forced’ to implement these methods.

Go has no inheritance. So if you want to call a method of an interface, the struct you actually call the method on must implement the method. Since your code can call Value on both a frame and an extraBallFrame, both structs have to implement Value. It is fine that extraBallFrame just delegates this call to the frame it contains.

Remember, Go is using composition, not inheritance. And I would not call the second ‘normal OO’ :slight_smile:

I don’t want to create a flame, it was just a bad expression.
Yes, Go does not have inheritance, that is why I used quotes do express myself. It is just easy to explain some problem using those concepts.

I understand it is OK to delegate it to another struct, and as I could learn it is the only way to do it, is it correct?

How about GetNext() and SetNext() on interface? Do I have another way to do the same? I just put it there because I can’t define fields/properties on interface, is it a good approach?

As far as I understand, these two methods will only every be called on a frame, right? So technically you are not forced to implement them for extraBallFrame, even if this leads to extraBallFrame to implementing frameValue.

extraBallFrame is a strange beast: It implements some methods of frameValue but not all. But you use it in places of a frameValue, hoping (knowing?) that the unimplemented methods will not be called.

This does not look like good design to me. I’d recommend to write unit tests that eventually call these two methods on an extraBallFrame instance and make the code robust enough to be abel to deal with this wrong use of the Game API.

That is exactly why I am asking feedback here, to validate my knowledge. Thanks for help me!

Maybe my concept is wrong but as I am using “composition” on extraBallFrame, I understand that it implements GetNext() and SetNext() methods also, not directly, but as “parent” methods.
As both methods just use a fields from frame, I don’t really needs to do something else on extraBallFrame.

In fact on methods CalculateScore() and Show(), they call GetNext(), because it is in a loop, so even on last frame, the one as extraBallFrame, it will be called.

I have a test file more complete, if you want I can post here also!

You are correct about Value and FirstBallValue being avaialbe on extraBallFrame. You can remove

func (e *extraBallFrame) Value() int {
	return e.frame.Value()
}
func (e *extraBallFrame) FirstBallValue() int {
	return e.frame.FirstBallValue()
}

and your program works as before.

Here is my test file.

package main

import (
	"testing"
)

func TestBownling270(t *testing.T)  {
	shouldBeValue := 270
	shouldBeString := "X,X,X,X,X,X,X,X,X,9/1"

	game := &Game{}
	game.AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddFrameExtra(9, 1, 1)
	
	asIsValue := game.CalculateScore()
	if shouldBeValue != asIsValue {
		t.Error("Expected:", shouldBeValue, "Result:", asIsValue)
	}

	asIsString := game.Show()
	if shouldBeString != asIsString {
		t.Error("Expected:", shouldBeString, "Result:", asIsString)
	}
}

func TestBownling271(t *testing.T)  {
	shouldBeValue := 271
	shouldBeString := "X,X,X,X,X,X,X,X,X,1/X"

	game := &Game{}
	game.AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddFrameExtra(1, 9, 10)
	
	asIsValue := game.CalculateScore()
	if shouldBeValue != asIsValue {
		t.Error("Expected:", shouldBeValue, "Result:", asIsValue)
	}

	asIsString := game.Show()
	if shouldBeString != asIsString {
		t.Error("Expected:", shouldBeString, "Result:", asIsString)
	}
}

func TestBownling300(t *testing.T)  {
	shouldBeValue := 300
	shouldBeString := "X,X,X,X,X,X,X,X,X,XXX"

	game := &Game{}
	game.AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddFrameExtra(10, 10, 10)
	
	asIsValue := game.CalculateScore()
	if shouldBeValue != asIsValue {
		t.Error("Expected:", shouldBeValue, "Result:", asIsValue)
	}

	asIsString := game.Show()
	if shouldBeString != asIsString {
		t.Error("Expected:", shouldBeString, "Result:", asIsString)
	}
}

func TestBownling0(t *testing.T)  {
	shouldBeValue := 0
	shouldBeString := "00,00,00,00,00,00,00,00,00,00"

	game := &Game{}
	game.AddMiss().AddMiss().AddMiss().AddMiss().AddMiss().AddMiss().AddMiss().AddMiss().AddMiss().AddMiss()
	
	asIsValue := game.CalculateScore()
	if shouldBeValue != asIsValue {
		t.Error("Expected:", shouldBeValue, "Result:", asIsValue)
	}

	asIsString := game.Show()
	if shouldBeString != asIsString {
		t.Error("Expected:", shouldBeString, "Result:", asIsString)
	}
}

func TestBownling10AsStrike(t *testing.T)  {
	shouldBeValue := 10
	shouldBeString := "X"

	game := &Game{}
	game.AddStrike()
	
	asIsValue := game.CalculateScore()
	if shouldBeValue != asIsValue {
		t.Error("Expected:", shouldBeValue, "Result:", asIsValue)
	}

	asIsString := game.Show()
	if shouldBeString != asIsString {
		t.Error("Expected:", shouldBeString, "Result:", asIsString)
	}
}

func TestBownling10AsSpare(t *testing.T)  {
	shouldBeValue := 10
	shouldBeString := "7/"

	game := &Game{}
	game.AddSpare(7)
	
	asIsValue := game.CalculateScore()
	if shouldBeValue != asIsValue {
		t.Error("Expected:", shouldBeValue, "Result:", asIsValue)
	}

	asIsString := game.Show()
	if shouldBeString != asIsString {
		t.Error("Expected:", shouldBeString, "Result:", asIsString)
	}
}

func TestBownling245(t *testing.T)  {
	shouldBeValue := 245
	shouldBeString := "X,X,X,X,X,X,X,X,X,11"

	game := &Game{}
	game.AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddFrame(1, 1)
	
	asIsValue := game.CalculateScore()
	if shouldBeValue != asIsValue {
		t.Error("Expected:", shouldBeValue, "Result:", asIsValue)
	}

	asIsString := game.Show()
	if shouldBeString != asIsString {
		t.Error("Expected:", shouldBeString, "Result:", asIsString)
	}
}

func TestBownling273(t *testing.T)  {
	shouldBeValue := 273
	shouldBeString := "X,X,X,X,X,X,X,X,X,X11"

	game := &Game{}
	game.AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddStrike().AddFrameExtra(10, 1, 1);
	
	asIsValue := game.CalculateScore()
	if shouldBeValue != asIsValue {
		t.Error("Expected:", shouldBeValue, "Result:", asIsValue)
	}

	asIsString := game.Show()
	if shouldBeString != asIsString {
		t.Error("Expected:", shouldBeString, "Result:", asIsString)
	}
}

func TestBownling20(t *testing.T)  {
	shouldBeValue := 20
	shouldBeString := "11,11,11,11,11,11,11,11,11,11"

	game := &Game{}
	game.AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1).AddFrame(1, 1)
	
	asIsValue := game.CalculateScore()
	if shouldBeValue != asIsValue {
		t.Error("Expected:", shouldBeValue, "Result:", asIsValue)
	}

	asIsString := game.Show()
	if shouldBeString != asIsString {
		t.Error("Expected:", shouldBeString, "Result:", asIsString)
	}
}

func TestBownling15(t *testing.T)  {
	shouldBeValue := 15
	shouldBeString := "11,1/,11"

	game := &Game{}
	game.AddFrame(1, 1).AddSpare(1).AddFrame(1, 1)
	
	asIsValue := game.CalculateScore()
	if shouldBeValue != asIsValue {
		t.Error("Expected:", shouldBeValue, "Result:", asIsValue)
	}

	asIsString := game.Show()
	if shouldBeString != asIsString {
		t.Error("Expected:", shouldBeString, "Result:", asIsString)
	}
}

Now I am confuse :slight_smile:
You are correct I can remove both methods and it still works, but it should not.
Let me try to explain how it is working:
CalculateScore -> extraBallFrame.Value() -> frame.Value() -> frame.SecondBallValue() -> extraBallFrame.SecondBallValue()

And that is exactly how it works in OO when we inherit and override a method.

But in Go it should be different, as we do not have the methos extraBallFrame.Value(), it should be like this:
CalculateScore -> extraBallFrame.Value() -> frame.Value() -> frame.SecondBallValue()

Am I right? What I am missing here?

It is said that the methods of the embedded struct frame are promoted to methods of the embedding struct extraBallFrame. This makes it possible to call Value and FirstBallValue on a an extraBallFrame instance even though only the embedded frame implements these methods.

This is different from OO as known from other languages. I recommend to take a look at Methods, Interfaces and Embedded Types in Go or to buy a copy of the excellent book The Go Programming Language. I’ve learned go from it and I can really recommend it.

1 Like

In fact it was a logic problem, I fixed the code and now it is working and calculating as expected.
And Go way of work is exactly as expected also, but the bad logic was masquerading it.

Now score logic is concentrated and simplified, so it is easy to see how Go work regarding composition and method override.

Thank you so much for your help, I am learning a little bit more every day.
This book is very good, I will put it in my wish list … :wink:

1 Like