error handling and tidying

6 files changed, 127 insertions, 71 deletions

diff --git a/calc.go b/calc.go
index e6897fe..1edfc9e 100644
--- a/calc.go
+++ b/calc.go
@@ -26,50 +26,83 @@ type Calculator struct {
 // the buffer is empty this simply pops from the stack. If the stack is empty,
 // this simply pushes to the stack.
 func (c *Calculator) swap() {
-	st := c.stack.pop()
-	if con := parseConstant(c.buf); con != nil {
-		c.stack.push(*con)
-	} else if f, err := strconv.ParseFloat(c.buf, 64); err == nil {
-		c.stack.push(f)
+	stackVal, err := c.stack.pop()
+	stackIsEmpty := err != nil
+
+	if v, err := c.parseBuffer(); err == nil {
+		c.stack.push(v)
 	}
-	if st != nil {
-		c.buf = printNum(*st)
-	} else {
+
+	if stackIsEmpty {
 		c.buf = ""
+	} else {
+		c.buf = printNum(stackVal)
 	}
 }
 
 // negate negates the number in the buffer, if any; or the bottom number on the
 // stack, if any.
 func (c *Calculator) negate() {
-	if con := parseConstant(c.buf); con != nil {
-		c.buf = printNum(-*con)
-	} else if f, err := strconv.ParseFloat(c.buf, 64); err == nil {
-		c.buf = printNum(-f)
+	if v, err := c.parseBuffer(); err == nil {
+		c.buf = printNum(-v)
 	} else if len(c.buf) == 0 && len(c.stack) > 0 {
 		c.stack[len(c.stack)-1] = -c.stack[len(c.stack)-1]
 	}
 }
 
-// performOp performs the specified arithmetic operation and returns nil or
-// OpError if op is not a valid operator.
-func (c *Calculator) performOp(op byte) error {
-	if len(c.stack) < 1 {
-		return nil
+// performOp performs the specified arithmetic operation.
+func (c *Calculator) performOperation(operator byte) error {
+	fn, err := parseOperator(operator)
+	if err != nil {
+		return err
 	}
 
-	fn, err := parseOp(op)
+	lhs, rhs, err := c.operands()
 	if err != nil {
 		return err
 	}
 
-	if con := parseConstant(c.buf); con != nil {
-		fn(&c.stack[len(c.stack)-1], *con)
-	} else if fl, err := strconv.ParseFloat(c.buf, 64); err == nil {
-		fn(&c.stack[len(c.stack)-1], fl)
-	} else if len(c.stack) > 1 {
-		fn(&c.stack[len(c.stack)-2], *c.stack.pop())
-	}
+	c.stack.push(fn(lhs, rhs))
 	c.buf = ""
 	return nil
 }
+
+// operands returns the operands of an arithmetic operation.
+func (c *Calculator) operands() (lhs, rhs float64, err error) {
+	if buf, err := c.parseBuffer(); err == nil {
+		rhs = buf
+		lhs, err = c.stack.pop()
+		return lhs, rhs, nil
+	} else if stk, err := c.stack.pop(); err == nil {
+		rhs = stk
+		if lhs, err = c.stack.pop(); err == nil {
+			return lhs, rhs, nil
+		}
+		c.stack.push(rhs)
+	} // not enough operands
+	return 0, 0, OperandErr{}
+}
+
+// parseBuffer returns the numerical value of the contents of the buffer.
+func (c Calculator) parseBuffer() (float64, error) {
+	if con, err := parseConstant(c.buf); err == nil {
+		return con, nil
+	} else if fl, err := strconv.ParseFloat(c.buf, 64); err == nil {
+		return fl, nil
+	}
+	return 0, InvalidBufferContentErr{c.buf}
+}
+
+type InvalidBufferContentErr struct {
+	buf string
+}
+
+func (e InvalidBufferContentErr) Error() string {
+	return "invalid buffer contents: " + e.buf
+}
+
+type OperandErr struct{}
+
+func (e OperandErr) Error() string {
+	return "not enough operands"
+}
diff --git a/const.go b/const.go
index ff0d09c..3808e82 100644
--- a/const.go
+++ b/const.go
@@ -2,16 +2,20 @@ package main
 
 import "math"
 
-// parseConstant returns nil if s is not a valid constant.
-func parseConstant(s string) *float64 {
+func parseConstant(s string) (float64, error) {
 	switch s {
 	case "pi":
-		// Assign to variable because can't take address of constant.
-		var pi float64 = math.Pi
-		return &pi
+		return math.Pi, nil
 	case "e":
-		var e float64 = math.E
-		return &e
+		return math.E, nil
 	}
-	return nil
+	return 0, InvalidConstantErr{s}
+}
+
+type InvalidConstantErr struct {
+	s string
+}
+
+func (e InvalidConstantErr) Error() string {
+	return "invalid constant: " + e.s
 }
diff --git a/func.go b/func.go
index 42763c0..c64e97a 100644
--- a/func.go
+++ b/func.go
@@ -73,32 +73,29 @@ func invTrig(fn string) func(Calculator) {
 
 // Convert radians to degrees.
 func deg(c Calculator) {
-	if len(c.stack) > 0 {
-		c.stack[len(c.stack)-1] = degrees(c.stack[len(c.stack)-1])
+	if n, err := c.stack.pop(); err == nil {
+		c.stack.push(degrees(n))
 	}
 }
 
 // Convert degrees to radians.
 func rad(c Calculator) {
-	if len(c.stack) > 0 {
-		c.stack[len(c.stack)-1] = radians(c.stack[len(c.stack)-1])
+	if n, err := c.stack.pop(); err == nil {
+		c.stack.push(radians(n))
 	}
 }
 
 // Factorial (!).
 func fac(c Calculator) {
-	if len(c.stack) > 0 {
-		a := &c.stack[len(c.stack)-1] // will replace with a!
-		if float64(int(*a)) != *a {   // undefined on non-ints
-			return
-		} else if int(*a) == 0 { // 0! = 1
-			*a = 1.0
-		} else { // a! = a*(a-1)!
-			for i := int(*a) - 1; i > 1; i-- {
-				*a *= float64(i)
-			}
-		}
+	n, err := c.stack.pop()
+	if err != nil {
+		return
+	}
+	if !isUint(n) { // undefined on non-ints
+		c.stack.push(n)
+		return
 	}
+	c.stack.push(float64(factorial(uint(n))))
 }
 
 // radians converts degrees to radians.
@@ -110,3 +107,19 @@ func radians(deg float64) float64 {
 func degrees(rad float64) float64 {
 	return rad * 180 / math.Pi
 }
+
+// factorial returns n! (n factorial).
+func factorial(n uint) uint {
+	if n == 0 { // 0! = 1
+		return 1
+	}
+	// n! = n*(n-1)!
+	for i := n - 1; i > 1; i-- {
+		n *= i
+	}
+	return n
+}
+
+func isUint(n float64) bool {
+	return float64(uint(n)) == n
+}
diff --git a/op.go b/op.go
index ab57a84..6ccc6a8 100644
--- a/op.go
+++ b/op.go
@@ -5,39 +5,41 @@ import (
 	"math"
 )
 
-// parseOp returns a closure that performs the specified arithmetic operation,
+// parseOperator returns a closure that performs the specified arithmetic operation,
 // or OpError if op is not a valid operator.
-func parseOp(op byte) (func(lhs *float64, rhs float64), error) {
+func parseOperator(op byte) (func(lhs float64, rhs float64) float64, error) {
 	switch op {
 	case '+':
-		return func(lhs *float64, rhs float64) { *lhs += rhs }, nil
+		return func(lhs, rhs float64) float64 { return lhs + rhs }, nil
 	case '-':
-		return func(lhs *float64, rhs float64) { *lhs -= rhs }, nil
+		return func(lhs, rhs float64) float64 { return lhs - rhs }, nil
 	case '*':
-		return func(lhs *float64, rhs float64) { *lhs *= rhs }, nil
+		return func(lhs, rhs float64) float64 { return lhs * rhs }, nil
 	case '/':
-		return func(lhs *float64, rhs float64) {
+		return func(lhs, rhs float64) float64 {
 			if rhs != 0 {
-				*lhs /= rhs
+				return lhs / rhs
 			}
+			return lhs
 		}, nil
 	case '%':
-		return func(lhs *float64, rhs float64) {
+		return func(lhs, rhs float64) float64 {
 			if rhs != 0 {
-				*lhs = float64(int64(*lhs) % int64(rhs))
+				return float64(int64(lhs) % int64(rhs))
 			}
+			return lhs
 		}, nil
 	case '^':
-		return func(lhs *float64, rhs float64) { *lhs = math.Pow(*lhs, rhs) }, nil
+		return func(lhs, rhs float64) float64 { return math.Pow(lhs, rhs) }, nil
 	}
-	return nil, OpError{op}
+	return nil, OperatorErr{op}
 }
 
-// OpError records an invalid arithmetic operator.
-type OpError struct {
+// OperatorErr records an invalid arithmetic operator.
+type OperatorErr struct {
 	c byte
 }
 
-func (e OpError) Error() string {
+func (e OperatorErr) Error() string {
 	return fmt.Sprintf("invalid operator: %c", e.c)
 }
diff --git a/stack.go b/stack.go
index 182d418..1a52e94 100644
--- a/stack.go
+++ b/stack.go
@@ -6,11 +6,17 @@ func (s *Stack) push(v float64) {
 	*s = append(*s, v)
 }
 
-func (s *Stack) pop() *float64 {
+func (s *Stack) pop() (float64, error) {
 	if len(*s) > 0 {
 		v := (*s)[len(*s)-1]
 		*s = (*s)[:len(*s)-1]
-		return &v
+		return v, nil
 	}
-	return nil
+	return 0, EmptyStackErr{}
+}
+
+type EmptyStackErr struct{}
+
+func (e EmptyStackErr) Error() string {
+	return "empty stack"
 }
diff --git a/ui.go b/ui.go
index 46f748e..d98577a 100644
--- a/ui.go
+++ b/ui.go
@@ -2,7 +2,6 @@ package main
 
 import (
 	"fmt"
-	"strconv"
 
 	"github.com/charmbracelet/bubbletea"
 )
@@ -54,7 +53,8 @@ func (ui UI) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		case "N":
 			ui.calc.negate()
 		case "+", "-", "*", "/", "%", "^":
-			if err := ui.calc.performOp(msg.String()[0]); err != nil {
+			operator := msg.String()[0]
+			if err := ui.calc.performOperation(operator); err != nil {
 				panic(err)
 			}
 		case "backspace":
@@ -64,10 +64,8 @@ func (ui UI) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		case "enter":
 			if fn := parseFunction(ui.calc.buf); fn != nil {
 				fn(ui.calc)
-			} else if con := parseConstant(ui.calc.buf); con != nil {
-				ui.calc.stack.push(*con)
-			} else if f, err := strconv.ParseFloat(ui.calc.buf, 64); err == nil {
-				ui.calc.stack.push(f)
+			} else if v, err := ui.calc.parseBuffer(); err == nil {
+				ui.calc.stack.push(v)
 			}
 			ui.calc.buf = ""
 		default: