reader - Add object field type router, refactor error to use wrap

Author: sheenobu

Diff for: obj/errors.go

@@ -1,25 +1,11 @@
 package obj
 
-import "fmt"
+import "github.com/pkg/errors"
 
-// ParseError represents an error in parsing
-type ParseError struct {
-	LineNumber int64
-	ItemType   string
-	Err        []error
+func wrapLineNumber(lineNumber int64, err error) error {
+	return errors.Wrapf(err, "error at line %d", lineNumber)
 }
 
-func (err *ParseError) Error() string {
-	return fmt.Sprintf("Parse error at line %d for %s: %s",
-		err.LineNumber,
-		err.ItemType,
-		err.Err)
-}
-
-func wrapParseErrors(lineNumber int64, itemType string, err ...error) error {
-	return &ParseError{
-		LineNumber: lineNumber,
-		ItemType:   itemType,
-		Err:        err,
-	}
+func wrapParseErrors(itemType string, err error) error {
+	return errors.Wrapf(err, "error parsing %s", itemType)
 }

Diff for: obj/reader.go

@@ -10,13 +10,24 @@ type Reader interface {
 	Read() (*Object, error)
 }
 
-// NewReader creates a new reader from the given io reader
+// NewReader creates a new reader frrouter the given io reader
 func NewReader(r io.Reader) Reader {
-	return &stdReader{r}
+	sr := &stdReader{
+		r:      r,
+		router: make(objectRouter),
+	}
+	sr.router["#"] = commentHandler
+	sr.router["o"] = objectHandler
+	sr.router["v"] = vertexHandler
+	sr.router["vn"] = normalHandler
+	sr.router["vt"] = textureHandler
+	sr.router["f"] = faceHandler
+	return sr
 }
 
 type stdReader struct {
-	r io.Reader
+	r      io.Reader
+	router objectRouter
 }
 
 func (r *stdReader) Read() (*Object, error) {
@@ -43,54 +54,79 @@ func (r *stdReader) Read() (*Object, error) {
 }
 
 func (r *stdReader) readLine(line []byte, lineNumber int64, o *Object) error {
-
-	//TODO: cyclomic complexity is 11. Would a 'router' be better here?
-
 	if len(line) == 0 {
 		return nil
 	}
 
 	tokens := splitByToken(line, ' ')
-	rest := tokens[1:]
 
-	switch string(tokens[0]) {
-	case "#":
-		// skip comments
-		return nil
-	case "o":
-		o.Name = string(tokens[1])
-	case "v":
-		v, err := parseVertex(rest)
-		if err != nil {
-			return wrapParseErrors(lineNumber, "vertex (v)", err)
-		}
-		o.Vertices = append(o.Vertices, v)
-		return nil
-	case "vn":
-		vn, err := parseNormal(rest)
-		if err != nil {
-			return wrapParseErrors(lineNumber, "vertexNormal (vn)", err)
-		}
-		o.Normals = append(o.Normals, vn)
-		return nil
-	case "vt":
-		vt, err := parseTextCoord(rest)
-		if err != nil {
-			return wrapParseErrors(lineNumber, "textureCoordinate (vt)", err)
-		}
+	if _, err := r.router.Route(o, tokens...); err != nil {
+		return wrapLineNumber(lineNumber, err)
+	}
 
-		o.Textures = append(o.Textures, vt)
-	case "f":
-		f, err := parseFace(rest, o)
-		if err != nil {
-			return wrapParseErrors(lineNumber, "face (f)", err)
-		}
-		o.Faces = append(o.Faces, f)
-		return nil
-	default:
-		//	fmt.Printf("ignoring token: %s\n", tokens[0])
-		return nil
+	return nil
+}
+
+func commentHandler(o *Object, rest ...[]byte) error {
+	return nil
+}
+
+func objectHandler(o *Object, rest ...[]byte) error {
+	o.Name = string(rest[0])
+	return nil
+}
+
+func vertexHandler(o *Object, rest ...[]byte) error {
+	v, err := parseVertex(rest)
+	if err != nil {
+		return wrapParseErrors("vertex (v)", err)
 	}
+	o.Vertices = append(o.Vertices, v)
+	return nil
+}
 
+func normalHandler(o *Object, rest ...[]byte) error {
+	vn, err := parseNormal(rest)
+	if err != nil {
+		return wrapParseErrors("vertexNormal (vn)", err)
+	}
+	o.Normals = append(o.Normals, vn)
 	return nil
 }
+
+func textureHandler(o *Object, rest ...[]byte) error {
+	vt, err := parseTextCoord(rest)
+	if err != nil {
+		return wrapParseErrors("textureCoordinate (vt)", err)
+	}
+
+	o.Textures = append(o.Textures, vt)
+	return nil
+}
+
+func faceHandler(o *Object, rest ...[]byte) error {
+	f, err := parseFace(rest, o)
+	if err != nil {
+		return wrapParseErrors("face (f)", err)
+	}
+	o.Faces = append(o.Faces, f)
+	return nil
+}
+
+type objectRouter map[string]func(*Object, ...[]byte) error
+
+// Route returns true if the list of tokens has been routed, false if it has been skipped
+func (router objectRouter) Route(o *Object, tokens ...[]byte) (bool, error) {
+	r, ok := router[string(tokens[0])]
+	if !ok {
+		return false, nil
+	}
+	rest := tokens[1:]
+
+	err := r(o, rest...)
+	if err != nil {
+		return false, err
+	}
+
+	return true, nil
+}

Diff for: obj/reader_test.go

@@ -33,20 +33,20 @@ var readLineTests = []struct {
 	{"#", ""},
 	{" #", ""},
 
-	{"vn x", "Parse error at line 0 for vertexNormal (vn): [item length is incorrect]"},
-	{"vt x", "Parse error at line 0 for textureCoordinate (vt): [item length is incorrect]"},
+	{"vn x", "error at line 0: error parsing vertexNormal (vn): item length is incorrect"},
+	{"vt x", "error at line 0: error parsing textureCoordinate (vt): item length is incorrect"},
 	{"v 0 0 0", ""},
-	{"v x", "Parse error at line 0 for vertex (v): [item length is incorrect]"},
-	{"v 0 x 0", "Parse error at line 0 for vertex (v): [unable to parse Y coordinate]"},
+	{"v x", "error at line 0: error parsing vertex (v): item length is incorrect"},
+	{"v 0 x 0", "error at line 0: error parsing vertex (v): unable to parse Y coordinate"},
 
 	{"vn 0 0 0", ""},
 
 	{"f 1", ""},
 
 	//TODO: better errors
-	{"f x", "Parse error at line 0 for face (f): [strconv.ParseInt: parsing \"x\": invalid syntax]"},
-	{"f 1/x/1", "Parse error at line 0 for face (f): [strconv.ParseInt: parsing \"x\": invalid syntax]"},
-	{"f 1/1/y", "Parse error at line 0 for face (f): [strconv.ParseInt: parsing \"y\": invalid syntax]"},
+	{"f x", "error at line 0: error parsing face (f): strconv.ParseInt: parsing \"x\": invalid syntax"},
+	{"f 1/x/1", "error at line 0: error parsing face (f): strconv.ParseInt: parsing \"x\": invalid syntax"},
+	{"f 1/1/y", "error at line 0: error parsing face (f): strconv.ParseInt: parsing \"y\": invalid syntax"},
 }
 
 func TestReadLine(t *testing.T) {