Error handling instead of nil-checks. Stub errors need to be improved.

bounds.go

@@ -7,24 +7,38 @@ import (
 	"strconv"
 )
 
+var boundFinder = regexp.MustCompile("([0-9]*)x([0-9]*)")
+
+// Bounds of an image.
 type Bounds struct {
 	Width, Height int
 }
 
-func BoundsFromString(bounds string) Bounds {
-	var x, y int
+// BoundsFromString creates a Bounds from strings of the form:
+//		"100x150"		->	Width 100, Height 150
+//		"x150"			->	Width   0, Height 150
+//		"100x"			->	Width 100, Height   0
+//		"x"					->	Width   0, Height   0
+//		"no match" 	-> nil, error
+func BoundsFromString(bounds string) (*Bounds, error) {
 
-	finder, err := regexp.Compile("([0-9]*)x([0-9]*)")
-	if err != nil {
-		return Bounds{0, 0}
+	dimensions := boundFinder.FindStringSubmatch(bounds)
+	if len(dimensions) != 3 {
+		return nil, fmt.Errorf("malformed bound string")
 	}
 
-	dimensions := finder.FindStringSubmatch(bounds)
-
-	x, _ = strconv.Atoi(dimensions[1])
-	y, _ = strconv.Atoi(dimensions[2])
+	atoiOrZero := func(str string) int {
+		if str == "" {
+			return 0
+		}
+		val, _ := strconv.Atoi(str)
+		return val
+	}
 
-	return Bounds{x, y}
+	return &Bounds{
+		Width:  atoiOrZero(dimensions[1]),
+		Height: atoiOrZero(dimensions[2]),
+	}, nil
 }
 
 func (b Bounds) String() string {

bounds_test.go

@@ -95,18 +95,50 @@ func TestGrow(t *testing.T) {
 	}
 }
 
+var stringTests = []struct {
+	name      string
+	bounds    string
+	want      *Bounds
+	shouldErr bool
+}{
+	{"Two proper sizes", "100x50", &Bounds{100, 50}, false},
+	{"Only height", "x50", &Bounds{0, 50}, false},
+	{"Only width", "100x", &Bounds{100, 0}, false},
+	{"An invalid bound", "not a bound", nil, true},
+}
+
 func TestFromString(t *testing.T) {
-  bounds := BoundsFromString("100x50")
-  if bounds.Width != 100 { t.Errorf("Width is wrong: %d", bounds.Width) }
-  if bounds.Height != 50 { t.Errorf("Height is wrong: %d", bounds.Height) }
+	for _, tt := range stringTests {
+		t.Logf("Extracting bounds: %s", tt.name)
+		bounds, err := BoundsFromString(tt.bounds)
 
-  bounds = BoundsFromString("x50")
-  if bounds.Width != 0 { t.Errorf("Width is wrong : %d", bounds.Width) }
-  if bounds.Height != 50 { t.Errorf("Height is wrong: %d", bounds.Height) }
+		switch {
 
-  bounds = BoundsFromString("50x")
-  if bounds.Width != 50 { t.Errorf("Width is wrong : %d", bounds.Width) }
-  if bounds.Height != 0 { t.Errorf("Height is wrong: %d", bounds.Height) }
+		case tt.shouldErr && err == nil:
+			t.Error("want an error, got nothing")
 
-}
+		case !tt.shouldErr && err != nil:
+			t.Errorf("want no error, got '%v'", err)
+
+		case tt.want == nil && bounds != nil:
+			t.Errorf("want nil bound, got '%#v'", bounds)
+
+		case tt.want != nil && bounds == nil:
+			t.Errorf("want '%#v', got nothing", tt.want)
 
+		}
+
+		if tt.shouldErr {
+			continue // tt.bounds will be nil
+		}
+
+		// not an error case, bounds is not nil
+		if tt.want.Width != bounds.Width {
+			t.Errorf("want width %d, got %d", tt.want.Width, bounds.Width)
+		}
+
+		if tt.want.Height != bounds.Height {
+			t.Errorf("want height %d, got %d", tt.want.Height, bounds.Height)
+		}
+	}
+}

gd.go

@@ -5,10 +5,7 @@ package mogrify
 import "C"
 
 import (
-	//	"bytes"
-	//	"fmt"
 	"errors"
-	//"log"
 	"unsafe"
 )
 

gif.go

@@ -2,6 +2,7 @@ package mogrify
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 )
 
@@ -10,15 +11,15 @@ type Gif struct {
 	GdImage
 }
 
-func DecodeGif(reader io.Reader) Image {
+func DecodeGif(reader io.Reader) (Image, error) {
 	var image Gif
 
 	image.gd = gdCreateFromGif(drain(reader))
 	if image.gd == nil {
-		return nil
+		return nil, fmt.Errorf("couldn't create GIF decoder")
 	}
 
-	return &image
+	return &image, nil
 }
 
 func EncodeGif(w io.Writer, img Image) (int64, error) {

image_test.go

@@ -16,10 +16,9 @@ func asset(asset string) Image {
 	file, _ := os.Open("./assets/image.jpg")
 	defer file.Close()
 
-	image := DecodeJpeg(file)
-
-	if image == nil {
-		panic("Image didnt load")
+	image, err := DecodeJpeg(file)
+	if err != nil {
+		panic("Image didnt load: " + err.Error())
 	}
 
 	return image
@@ -62,7 +61,7 @@ func TestCropSuccess(t *testing.T) {
 	img := asset("./assets/image.jpg")
 	defer img.Destroy()
 
-  cropped, err := img.NewCropped(0,0,Bounds{50, 50})
+	cropped, err := img.NewCropped(0, 0, Bounds{50, 50})
 	if err != nil {
 		t.Error(err)
 	}
@@ -118,7 +117,7 @@ func TestCropFailure(t *testing.T) {
 	img := asset("./assets/image.jpg")
 	defer img.Destroy()
 
-	_, err := img.NewCropped(0,0,Bounds{-1, 50})
+	_, err := img.NewCropped(0, 0, Bounds{-1, 50})
 	if err == nil {
 		t.Fatalf("This should have failed...")
 	}
@@ -149,7 +148,10 @@ func TestDecodeEncode(t *testing.T) {
 		return
 	}
 
-	roundtrip := DecodeJpeg(&buffer)
+	roundtrip, err := DecodeJpeg(&buffer)
+	if err != nil {
+		panic("Couldn't load image: " + err.Error())
+	}
 
 	assertDimension(t, roundtrip, "100x100")
 }

jpeg.go

@@ -2,6 +2,7 @@ package mogrify
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 )
 
@@ -10,15 +11,15 @@ type Jpeg struct {
 	GdImage
 }
 
-func DecodeJpeg(reader io.Reader) Image {
+func DecodeJpeg(reader io.Reader) (Image, error) {
 	var image Jpeg
 
 	image.gd = gdCreateFromJpeg(drain(reader))
 	if image.gd == nil {
-		return nil
+		return nil, fmt.Errorf("couldn't create JPEG decoder")
 	}
 
-	return &image
+	return &image, nil
 }
 
 func EncodeJpeg(w io.Writer, img Image) (int64, error) {

png.go

@@ -2,6 +2,7 @@ package mogrify
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 )
 
@@ -10,15 +11,15 @@ type Png struct {
 	GdImage
 }
 
-func DecodePng(reader io.Reader) Image {
+func DecodePng(reader io.Reader) (Image, error) {
 	var image Png
 
 	image.gd = gdCreateFromPng(drain(reader))
 	if image.gd == nil {
-		return nil
+		return nil, fmt.Errorf("couldn't create PNG decoder")
 	}
 
-	return &image
+	return &image, nil
 }
 
 func EncodePng(w io.Writer, img Image) (int64, error) {

registry.go

@@ -1,17 +1,16 @@
 package mogrify
 
 import (
-	"errors"
+	"fmt"
 	"io"
 )
 
-type decodeFunc (func(io.Reader) Image)
-type encodeFunc (func(io.Writer, Image) (int64, error))
+type decodeFunc func(io.Reader) (Image, error)
+type encodeFunc func(io.Writer, Image) (int64, error)
 
 var (
-	noEncoder                       = errors.New("no encoder for mime type")
-	encoders  map[string]encodeFunc = make(map[string]encodeFunc)
-	decoders  map[string]decodeFunc = make(map[string]decodeFunc)
+	encoders = make(map[string]encodeFunc)
+	decoders = make(map[string]decodeFunc)
 )
 
 func registerFormat(mime string, e encodeFunc, d decodeFunc) {
@@ -26,22 +25,22 @@ func init() {
 	registerFormat("image/gif", EncodeGif, DecodeGif)
 }
 
+// Encode an image onto a writer using an encoder appropriate for the
+// given mimetype, if one exists.
 func Encode(mime string, w io.Writer, i Image) (int64, error) {
-	encoder := encoders[mime]
-
-	if encoder == nil {
-		return 0, noEncoder
+	encoder, ok := encoders[mime]
+	if !ok {
+		return 0, fmt.Errorf("no encoder for mime type '%s'", mime)
 	}
-
 	return encoder(w, i)
 }
 
-func Decode(mime string, r io.Reader) Image {
-	decoder := decoders[mime]
-
-	if decoder == nil {
-		return nil
+// Decode an image from the reader using a decoder appropriate for the
+// given mimetype, if one exists.
+func Decode(mime string, r io.Reader) (Image, error) {
+	decoder, ok := decoders[mime]
+	if !ok {
+		return nil, fmt.Errorf("no decoder for mime type '%s'", mime)
 	}
-
 	return decoder(r)
 }