Clean up code (#20)

cmd/root.go

@@ -14,7 +14,6 @@ import (
 	"github.com/spenserblack/gh-collab-montage/pkg/avatar/grid"
 	"github.com/spenserblack/gh-collab-montage/pkg/usersource"
 	"github.com/spf13/cobra"
-	"golang.org/x/image/draw"
 )
 
 var rootCmd = &cobra.Command{
@@ -23,8 +22,23 @@ var rootCmd = &cobra.Command{
 	Run: func(cmd *cobra.Command, args []string) {
 		err := repoFlag.fillWithDefault()
 		onError(err)
-		avatar.Width = avatarSize
-		avatar.Height = avatarSize
+
+		var formatter avatar.Formatter
+		switch avatarStyle.String() {
+		case "circle":
+			formatter = avatar.Circlify
+		case "square":
+			formatter = avatar.Noop
+		default:
+			panic("unreachable: invalid avatar style")
+		}
+
+		g := &grid.Grid{
+			AvatarSize: avatarSize,
+			Margin:     margin,
+			Formatter:  formatter,
+		}
+
 		f, err := os.Create("montage.png")
 		onError(err)
 		defer f.Close()
@@ -41,24 +55,11 @@ var rootCmd = &cobra.Command{
 			if user.Type != "User" {
 				continue
 			}
-			a, err := avatar.Decode(user.AvatarURL)
+			a, err := avatar.DecodeFromURL(user.AvatarURL)
 			onError(err)
-			// TODO Expose this to users
-			resized := image.NewRGBA(image.Rect(0, 0, avatar.Width, avatar.Height))
-			draw.ApproxBiLinear.Scale(resized, resized.Bounds(), a, a.Bounds(), draw.Src, nil)
-			avatars = append(avatars, resized)
-		}
-
-		var formatter avatar.Formatter
-		switch avatarStyle.String() {
-		case "circle":
-			formatter = avatar.Circlify
-		case "square":
-			formatter = avatar.Noop
-		default:
-			panic("unreachable: invalid avatar style")
+			avatars = append(avatars, a)
 		}
-		g := grid.NewWithSize(len(avatars), margin, formatter)
+		g.WithSize(len(avatars))
 		for _, a := range avatars {
 			g.AddAvatar(a)
 		}

pkg/avatar/avatar.go

@@ -1,13 +1,2 @@
 // Package avatar provides utilities for GitHub avatars
 package avatar
-
-var (
-	// Width is a global value for the width of an avatar in pixels.
-	//
-	// Several utilities share this value.
-	Width = 400
-	// Height is a global value for the height of an avatar in pixels.
-	//
-	// Several utilities share this value.
-	Height = 400
-)

pkg/avatar/decode.go

@@ -8,8 +8,10 @@ import (
 	"net/http"
 )
 
-// Decode decodes a GitHub avatar from a URL.
-func Decode(url string) (image.Image, error) {
+// DecodeFromURL decodes an image GitHub avatar from a URL.
+//
+// This can be used to get an avatar from GitHub.
+func DecodeFromURL(url string) (image.Image, error) {
 	resp, err := http.Get(url)
 	if err != nil {
 		return nil, err

pkg/avatar/grid/draw.go

@@ -11,20 +11,26 @@ import (
 // AddAvatar adds an avatar's image to a grid.
 //
 // If needed, it expands the size of the underlying image.
-func (g *AvatarGrid) AddAvatar(avatar image.Image) {
-	// TODO Assert that avatars are the appropriate size?
-	formatted := g.formatter(avatar)
+func (g *Grid) AddAvatar(avatar image.Image) {
+	resized := image.NewRGBA(image.Rect(0, 0, g.AvatarSize, g.AvatarSize))
+	draw.ApproxBiLinear.Scale(resized, resized.Bounds(), avatar, avatar.Bounds(), draw.Src, nil)
+
+	formatter := g.Formatter
+	if formatter == nil {
+		formatter = av.Noop
+	}
+	formatted := formatter(resized)
 	// NOTE g.col and g.row are 0-indexed
 	if g.row == 0 && g.Cols() <= g.col {
 		g.setBounds(g.Rows(), g.Cols()+1)
 	} else if g.col == 0 && g.Rows() <= g.row {
 		g.setBounds(g.Rows()+1, g.Cols())
 	}
-	x := (g.col * av.Width) + (g.col * g.margin)
-	y := (g.row * av.Height) + (g.row * g.margin)
+	x := (g.col * g.AvatarSize) + (g.col * g.Margin)
+	y := (g.row * g.AvatarSize) + (g.row * g.Margin)
 	draw.Draw(
 		g.image,
-		image.Rect(x, y, x+av.Width, y+av.Height),
+		image.Rect(x, y, x+g.AvatarSize, y+g.AvatarSize),
 		formatted,
 		image.Point{},
 		draw.Src,

pkg/avatar/grid/grid.go

@@ -13,32 +13,31 @@ import (
 // PerRow is the number of avatars to draw per row.
 const perRow = 10
 
-// AvatarGrid is a grid of GitHub avatars.
+// Grid is a grid of GitHub avatars.
 //
 // It expands and adds new rows when needed.
-type AvatarGrid struct {
+type Grid struct {
+	// AvatarSize is the size of each avatar in the grid.
+	AvatarSize int
+	// Margin is the number of pixels between avatars.
+	Margin int
+	// Formatter is a function to call on avatar images to format them.
+	Formatter avatar.Formatter
+	// Image is the underlying image of the grid.
 	image draw.Image
 	// Row is the current row (0-indexed).
 	row int
 	// Col is the current column (0-indexed).
 	col int
-	// Margin is the number of pixels between avatars.
-	margin int
 	// Cols is the number of columns in the grid.
 	cols int
 	// Rows is the number of rows in the grid.
 	rows int
-	// Formatter is a function to call on avatar images to format them.
-	formatter avatar.Formatter
 }
 
-// New returns a new AvatarGrid.
-func New(margin int, formatter avatar.Formatter) *AvatarGrid {
-	return NewWithSize(0, margin, formatter)
-}
-
-// NewWithSize returns a new AvatarGrid with the given size.
-func NewWithSize(avatars int, margin int, formatter avatar.Formatter) *AvatarGrid {
+// WithSize updates the underlying image of the grid to fit the given number of
+// avatars. This can help prevent frequent resizing of the underlying image.
+func (g *Grid) WithSize(avatars int) {
 	var cols, rows int
 	if avatars == 0 {
 		rows = 1
@@ -55,33 +54,27 @@ func NewWithSize(avatars int, margin int, formatter avatar.Formatter) *AvatarGri
 	} else {
 		cols = avatars
 	}
-	g := &AvatarGrid{
-		margin:    margin,
-		cols:      cols,
-		rows:      rows,
-		formatter: formatter,
-	}
+	g.cols, g.rows = cols, rows
 	g.image = g.newDst()
-	return g
 }
 
 // Image returns the image of the grid.
-func (g AvatarGrid) Image() image.Image {
+func (g Grid) Image() image.Image {
 	return g.image
 }
 
 // Cols returns the number of columns in the grid.
-func (g AvatarGrid) Cols() int {
+func (g Grid) Cols() int {
 	return g.cols
 }
 
 // Rows returns the number of rows in the grid.
-func (g AvatarGrid) Rows() int {
+func (g Grid) Rows() int {
 	return g.rows
 }
 
 // SetBounds changes the bounds of the underlying image.
-func (g *AvatarGrid) setBounds(rows, cols int) {
+func (g *Grid) setBounds(rows, cols int) {
 	g.cols = cols
 	g.rows = rows
 	newImage := g.newDst()
@@ -90,23 +83,23 @@ func (g *AvatarGrid) setBounds(rows, cols int) {
 }
 
 // NewDst creates a new destination image based on the grid's dimensions.
-func (g AvatarGrid) newDst() draw.Image {
-	width := g.cols*avatar.Width + (g.cols-1)*g.margin
-	height := g.rows*avatar.Height + (g.rows-1)*g.margin
+func (g Grid) newDst() draw.Image {
+	width := g.cols*g.AvatarSize + (g.cols-1)*g.Margin
+	height := g.rows*g.AvatarSize + (g.rows-1)*g.Margin
 	return image.NewRGBA(image.Rect(0, 0, width, height))
 }
 
 // ColorModel returns the color model of the underlying image.
-func (g AvatarGrid) ColorModel() color.Model {
+func (g Grid) ColorModel() color.Model {
 	return g.image.ColorModel()
 }
 
 // Bounds returns the bounds of the underlying image.
-func (g AvatarGrid) Bounds() image.Rectangle {
+func (g Grid) Bounds() image.Rectangle {
 	return g.image.Bounds()
 }
 
 // At returns the color of the pixel at (x, y).
-func (g AvatarGrid) At(x, y int) color.Color {
+func (g Grid) At(x, y int) color.Color {
 	return g.image.At(x, y)
 }

pkg/avatar/grid/grid_test.go

@@ -8,17 +8,7 @@ import (
 	av "github.com/spenserblack/gh-collab-montage/pkg/avatar"
 )
 
-func TestNew(t *testing.T) {
-	g := New(100, av.Noop)
-	if g.Cols() != 0 {
-		t.Errorf("g.Cols() = %d, want 0", g.Cols())
-	}
-	if g.Rows() != 1 {
-		t.Errorf("g.Rows() = %d, want 1", g.Rows())
-	}
-}
-
-func TestNewWithSize(t *testing.T) {
+func TestWithSize(t *testing.T) {
 	tests := []struct {
 		name    string
 		avatars int
@@ -59,7 +49,12 @@ func TestNewWithSize(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			g := NewWithSize(tt.avatars, 100, av.Noop)
+			g := &Grid{
+				AvatarSize: 400,
+				Margin:     100,
+				Formatter:  av.Noop,
+			}
+			g.WithSize(tt.avatars)
 			if g.Cols() != tt.cols {
 				t.Errorf("g.Cols() = %d, want %d", g.Cols(), tt.cols)
 			}
@@ -174,7 +169,12 @@ func TestGrid_AddAvatar(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(fmt.Sprintf("%d avatars added to %d-avatar grid", tt.n, tt.size), func(t *testing.T) {
-			g := NewWithSize(tt.size, 100, av.Noop)
+			g := &Grid{
+				AvatarSize: 400,
+				Margin:     100,
+				Formatter:  av.Noop,
+			}
+			g.WithSize(tt.size)
 			for i := 0; i < tt.n; i++ {
 				g.AddAvatar(avatar)
 			}
@@ -186,5 +186,17 @@ func TestGrid_AddAvatar(t *testing.T) {
 			}
 		})
 	}
+}
 
+// Tests that the Noop formatter is used when no formatter is provided.
+func TestGrid_AddAvatar_nil_formatter(t *testing.T) {
+	avatar := image.NewAlpha(image.Rect(0, 0, 500, 500))
+	g := &Grid{
+		AvatarSize: 400,
+		Margin:     100,
+	}
+	g.WithSize(1)
+	g.AddAvatar(avatar)
+	// NOTE Basically if we didn't panic from a nil pointer dereference, we're good
+	// TODO Test grid's pixels by drawing images with known colors
 }