- formula engine: reduce cyclomatic complexity

- styles: allow empty and default cell formats, #628

calc.go

@@ -63,8 +63,8 @@ type formulaArg struct {
 type formulaFuncs struct{}
 
 // CalcCellValue provides a function to get calculated cell value. This
-// feature is currently in beta. Array formula, table formula and some other
+// feature is currently in working processing. Array formula, table formula
-// formulas are not supported currently.
+// and some other formulas are not supported currently.
 func (f *File) CalcCellValue(sheet, cell string) (result string, err error) {
 	var (
 		formula string
@@ -265,6 +265,89 @@ func (f *File) evalInfixExp(sheet string, tokens []efp.Token) (efp.Token, error)
 	return opdStack.Peek().(efp.Token), err
 }
 
+// calcAdd evaluate addition arithmetic operations.
+func calcAdd(opdStack *Stack) error {
+	if opdStack.Len() < 2 {
+		return errors.New("formula not valid")
+	}
+	rOpd := opdStack.Pop().(efp.Token)
+	lOpd := opdStack.Pop().(efp.Token)
+	lOpdVal, err := strconv.ParseFloat(lOpd.TValue, 64)
+	if err != nil {
+		return err
+	}
+	rOpdVal, err := strconv.ParseFloat(rOpd.TValue, 64)
+	if err != nil {
+		return err
+	}
+	result := lOpdVal + rOpdVal
+	opdStack.Push(efp.Token{TValue: fmt.Sprintf("%g", result), TType: efp.TokenTypeOperand, TSubType: efp.TokenSubTypeNumber})
+	return nil
+}
+
+// calcAdd evaluate subtraction arithmetic operations.
+func calcSubtract(opdStack *Stack) error {
+	if opdStack.Len() < 2 {
+		return errors.New("formula not valid")
+	}
+	rOpd := opdStack.Pop().(efp.Token)
+	lOpd := opdStack.Pop().(efp.Token)
+	lOpdVal, err := strconv.ParseFloat(lOpd.TValue, 64)
+	if err != nil {
+		return err
+	}
+	rOpdVal, err := strconv.ParseFloat(rOpd.TValue, 64)
+	if err != nil {
+		return err
+	}
+	result := lOpdVal - rOpdVal
+	opdStack.Push(efp.Token{TValue: fmt.Sprintf("%g", result), TType: efp.TokenTypeOperand, TSubType: efp.TokenSubTypeNumber})
+	return nil
+}
+
+// calcAdd evaluate multiplication arithmetic operations.
+func calcMultiply(opdStack *Stack) error {
+	if opdStack.Len() < 2 {
+		return errors.New("formula not valid")
+	}
+	rOpd := opdStack.Pop().(efp.Token)
+	lOpd := opdStack.Pop().(efp.Token)
+	lOpdVal, err := strconv.ParseFloat(lOpd.TValue, 64)
+	if err != nil {
+		return err
+	}
+	rOpdVal, err := strconv.ParseFloat(rOpd.TValue, 64)
+	if err != nil {
+		return err
+	}
+	result := lOpdVal * rOpdVal
+	opdStack.Push(efp.Token{TValue: fmt.Sprintf("%g", result), TType: efp.TokenTypeOperand, TSubType: efp.TokenSubTypeNumber})
+	return nil
+}
+
+// calcAdd evaluate division arithmetic operations.
+func calcDivide(opdStack *Stack) error {
+	if opdStack.Len() < 2 {
+		return errors.New("formula not valid")
+	}
+	rOpd := opdStack.Pop().(efp.Token)
+	lOpd := opdStack.Pop().(efp.Token)
+	lOpdVal, err := strconv.ParseFloat(lOpd.TValue, 64)
+	if err != nil {
+		return err
+	}
+	rOpdVal, err := strconv.ParseFloat(rOpd.TValue, 64)
+	if err != nil {
+		return err
+	}
+	result := lOpdVal / rOpdVal
+	if rOpdVal == 0 {
+		return errors.New(formulaErrorDIV)
+	}
+	opdStack.Push(efp.Token{TValue: fmt.Sprintf("%g", result), TType: efp.TokenTypeOperand, TSubType: efp.TokenSubTypeNumber})
+	return nil
+}
+
 // calculate evaluate basic arithmetic operations.
 func calculate(opdStack *Stack, opt efp.Token) error {
 	if opt.TValue == "-" && opt.TType == efp.TokenTypeOperatorPrefix {
@@ -279,80 +362,69 @@ func calculate(opdStack *Stack, opt efp.Token) error {
 		result := 0 - opdVal
 		opdStack.Push(efp.Token{TValue: fmt.Sprintf("%g", result), TType: efp.TokenTypeOperand, TSubType: efp.TokenSubTypeNumber})
 	}
+
 	if opt.TValue == "+" {
-		if opdStack.Len() < 2 {
+		if err := calcAdd(opdStack); err != nil {
-			return errors.New("formula not valid")
-		}
-		rOpd := opdStack.Pop().(efp.Token)
-		lOpd := opdStack.Pop().(efp.Token)
-		lOpdVal, err := strconv.ParseFloat(lOpd.TValue, 64)
-		if err != nil {
 			return err
 		}
-		rOpdVal, err := strconv.ParseFloat(rOpd.TValue, 64)
-		if err != nil {
-			return err
-		}
-		result := lOpdVal + rOpdVal
-		opdStack.Push(efp.Token{TValue: fmt.Sprintf("%g", result), TType: efp.TokenTypeOperand, TSubType: efp.TokenSubTypeNumber})
 	}
 	if opt.TValue == "-" && opt.TType == efp.TokenTypeOperatorInfix {
-		if opdStack.Len() < 2 {
+		if err := calcSubtract(opdStack); err != nil {
-			return errors.New("formula not valid")
-		}
-		rOpd := opdStack.Pop().(efp.Token)
-		lOpd := opdStack.Pop().(efp.Token)
-		lOpdVal, err := strconv.ParseFloat(lOpd.TValue, 64)
-		if err != nil {
 			return err
 		}
-		rOpdVal, err := strconv.ParseFloat(rOpd.TValue, 64)
-		if err != nil {
-			return err
-		}
-		result := lOpdVal - rOpdVal
-		opdStack.Push(efp.Token{TValue: fmt.Sprintf("%g", result), TType: efp.TokenTypeOperand, TSubType: efp.TokenSubTypeNumber})
 	}
 	if opt.TValue == "*" {
-		if opdStack.Len() < 2 {
+		if err := calcMultiply(opdStack); err != nil {
-			return errors.New("formula not valid")
-		}
-		rOpd := opdStack.Pop().(efp.Token)
-		lOpd := opdStack.Pop().(efp.Token)
-		lOpdVal, err := strconv.ParseFloat(lOpd.TValue, 64)
-		if err != nil {
 			return err
 		}
-		rOpdVal, err := strconv.ParseFloat(rOpd.TValue, 64)
-		if err != nil {
-			return err
-		}
-		result := lOpdVal * rOpdVal
-		opdStack.Push(efp.Token{TValue: fmt.Sprintf("%g", result), TType: efp.TokenTypeOperand, TSubType: efp.TokenSubTypeNumber})
 	}
 	if opt.TValue == "/" {
-		if opdStack.Len() < 2 {
+		if err := calcDivide(opdStack); err != nil {
-			return errors.New("formula not valid")
-		}
-		rOpd := opdStack.Pop().(efp.Token)
-		lOpd := opdStack.Pop().(efp.Token)
-		lOpdVal, err := strconv.ParseFloat(lOpd.TValue, 64)
-		if err != nil {
 			return err
 		}
-		rOpdVal, err := strconv.ParseFloat(rOpd.TValue, 64)
-		if err != nil {
-			return err
-		}
-		result := lOpdVal / rOpdVal
-		if rOpdVal == 0 {
-			return errors.New(formulaErrorDIV)
-		}
-		opdStack.Push(efp.Token{TValue: fmt.Sprintf("%g", result), TType: efp.TokenTypeOperand, TSubType: efp.TokenSubTypeNumber})
 	}
 	return nil
 }
 
+// parseOperatorPrefixToken parse operator prefix token.
+func (f *File) parseOperatorPrefixToken(optStack, opdStack *Stack, token efp.Token) (err error) {
+	if optStack.Len() == 0 {
+		optStack.Push(token)
+	} else {
+		tokenPriority := getPriority(token)
+		topOpt := optStack.Peek().(efp.Token)
+		topOptPriority := getPriority(topOpt)
+		if tokenPriority > topOptPriority {
+			optStack.Push(token)
+		} else {
+			for tokenPriority <= topOptPriority {
+				optStack.Pop()
+				if err = calculate(opdStack, topOpt); err != nil {
+					return
+				}
+				if optStack.Len() > 0 {
+					topOpt = optStack.Peek().(efp.Token)
+					topOptPriority = getPriority(topOpt)
+					continue
+				}
+				break
+			}
+			optStack.Push(token)
+		}
+	}
+	return
+}
+
+// isOperatorPrefixToken determine if the token is parse operator prefix
+// token.
+func isOperatorPrefixToken(token efp.Token) bool {
+	if (token.TValue == "-" && token.TType == efp.TokenTypeOperatorPrefix) ||
+		token.TValue == "+" || token.TValue == "-" || token.TValue == "*" || token.TValue == "/" {
+		return true
+	}
+	return false
+}
+
 // parseToken parse basic arithmetic operator priority and evaluate based on
 // operators and operands.
 func (f *File) parseToken(sheet string, token efp.Token, opdStack, optStack *Stack) error {
@@ -369,30 +441,9 @@ func (f *File) parseToken(sheet string, token efp.Token, opdStack, optStack *Sta
 		token.TType = efp.TokenTypeOperand
 		token.TSubType = efp.TokenSubTypeNumber
 	}
-	if (token.TValue == "-" && token.TType == efp.TokenTypeOperatorPrefix) || token.TValue == "+" || token.TValue == "-" || token.TValue == "*" || token.TValue == "/" {
+	if isOperatorPrefixToken(token) {
-		if optStack.Len() == 0 {
+		if err := f.parseOperatorPrefixToken(optStack, opdStack, token); err != nil {
-			optStack.Push(token)
+			return err
-		} else {
-			tokenPriority := getPriority(token)
-			topOpt := optStack.Peek().(efp.Token)
-			topOptPriority := getPriority(topOpt)
-			if tokenPriority > topOptPriority {
-				optStack.Push(token)
-			} else {
-				for tokenPriority <= topOptPriority {
-					optStack.Pop()
-					if err := calculate(opdStack, topOpt); err != nil {
-						return err
-					}
-					if optStack.Len() > 0 {
-						topOpt = optStack.Peek().(efp.Token)
-						topOptPriority = getPriority(topOpt)
-						continue
-					}
-					break
-				}
-				optStack.Push(token)
-			}
 		}
 	}
 	if token.TType == efp.TokenTypeSubexpression && token.TSubType == efp.TokenSubTypeStart { // (
@@ -461,11 +512,44 @@ func (f *File) parseReference(sheet, reference string) (result []string, matrix
 	return
 }
 
+// prepareValueRange prepare value range.
+func prepareValueRange(cr cellRange, valueRange []int) {
+	if cr.From.Row < valueRange[0] {
+		valueRange[0] = cr.From.Row
+	}
+	if cr.From.Col < valueRange[2] {
+		valueRange[2] = cr.From.Col
+	}
+	if cr.To.Row > valueRange[0] {
+		valueRange[1] = cr.To.Row
+	}
+	if cr.To.Col > valueRange[3] {
+		valueRange[3] = cr.To.Col
+	}
+}
+
+// prepareValueRef prepare value reference.
+func prepareValueRef(cr cellRef, valueRange []int) {
+	if cr.Row < valueRange[0] {
+		valueRange[0] = cr.Row
+	}
+	if cr.Col < valueRange[2] {
+		valueRange[2] = cr.Col
+	}
+	if cr.Row > valueRange[0] {
+		valueRange[1] = cr.Row
+	}
+	if cr.Col > valueRange[3] {
+		valueRange[3] = cr.Col
+	}
+}
+
 // rangeResolver extract value as string from given reference and range list.
-// This function will not ignore the empty cell. For example,
+// This function will not ignore the empty cell. For example, A1:A2:A2:B3 will
-// A1:A2:A2:B3 will be reference A1:B3.
+// be reference A1:B3.
 func (f *File) rangeResolver(cellRefs, cellRanges *list.List) (result []string, matrix [][]string, err error) {
-	var fromRow, toRow, fromCol, toCol int = 1, 1, 1, 1
+	// value range order: from row, to row, from column, to column
+	valueRange := []int{1, 1, 1, 1}
 	var sheet string
 	filter := map[string]string{}
 	// prepare value range
@@ -476,18 +560,7 @@ func (f *File) rangeResolver(cellRefs, cellRanges *list.List) (result []string,
 		}
 		rng := []int{cr.From.Col, cr.From.Row, cr.To.Col, cr.To.Row}
 		sortCoordinates(rng)
-		if cr.From.Row < fromRow {
+		prepareValueRange(cr, valueRange)
-			fromRow = cr.From.Row
-		}
-		if cr.From.Col < fromCol {
-			fromCol = cr.From.Col
-		}
-		if cr.To.Row > fromRow {
-			toRow = cr.To.Row
-		}
-		if cr.To.Col > toCol {
-			toCol = cr.To.Col
-		}
 		if cr.From.Sheet != "" {
 			sheet = cr.From.Sheet
 		}
@@ -497,24 +570,13 @@ func (f *File) rangeResolver(cellRefs, cellRanges *list.List) (result []string,
 		if cr.Sheet != "" {
 			sheet = cr.Sheet
 		}
-		if cr.Row < fromRow {
+		prepareValueRef(cr, valueRange)
-			fromRow = cr.Row
-		}
-		if cr.Col < fromCol {
-			fromCol = cr.Col
-		}
-		if cr.Row > fromRow {
-			toRow = cr.Row
-		}
-		if cr.Col > toCol {
-			toCol = cr.Col
-		}
 	}
 	// extract value from ranges
 	if cellRanges.Len() > 0 {
-		for row := fromRow; row <= toRow; row++ {
+		for row := valueRange[0]; row <= valueRange[1]; row++ {
 			var matrixRow = []string{}
-			for col := fromCol; col <= toCol; col++ {
+			for col := valueRange[2]; col <= valueRange[3]; col++ {
 				var cell, value string
 				if cell, err = CoordinatesToCellName(col, row); err != nil {
 					return
@@ -672,28 +734,15 @@ func (fn *formulaFuncs) ARABIC(argsList *list.List) (result string, err error) {
 		err = errors.New("ARABIC requires 1 numeric argument")
 		return
 	}
+	charMap := map[rune]float64{'I': 1, 'V': 5, 'X': 10, 'L': 50, 'C': 100, 'D': 500, 'M': 1000}
 	val, last, prefix := 0.0, 0.0, 1.0
 	for _, char := range argsList.Front().Value.(formulaArg).Value {
 		digit := 0.0
-		switch char {
+		if char == '-' {
-		case '-':
 			prefix = -1
 			continue
-		case 'I':
-			digit = 1
-		case 'V':
-			digit = 5
-		case 'X':
-			digit = 10
-		case 'L':
-			digit = 50
-		case 'C':
-			digit = 100
-		case 'D':
-			digit = 500
-		case 'M':
-			digit = 1000
 		}
+		digit, _ = charMap[char]
 		val += digit
 		switch {
 		case last == digit && (last == 5 || last == 50 || last == 500):
@@ -850,7 +899,7 @@ func (fn *formulaFuncs) BASE(argsList *list.List) (result string, err error) {
 		return
 	}
 	if radix < 2 || radix > 36 {
-		err = errors.New("radix must be an integer ≥ 2 and ≤ 36")
+		err = errors.New("radix must be an integer >= 2 and <= 36")
 		return
 	}
 	if argsList.Len() > 2 {

calc_test.go

@@ -381,7 +381,7 @@ func TestCalcCellValue(t *testing.T) {
 		// BASE
 		"=BASE()":        "BASE requires at least 2 arguments",
 		"=BASE(1,2,3,4)": "BASE allows at most 3 arguments",
-		"=BASE(1,1)":     "radix must be an integer ≥ 2 and ≤ 36",
+		"=BASE(1,1)":     "radix must be an integer >= 2 and <= 36",
 		// CEILING
 		"=CEILING()":      "CEILING requires at least 1 argument",
 		"=CEILING(1,2,3)": "CEILING allows at most 2 arguments",

cell.go

@@ -730,9 +730,9 @@ func (f *File) formattedValue(s int, v string) string {
 		return v
 	}
 	styleSheet := f.stylesReader()
-	ok := builtInNumFmtFunc[styleSheet.CellXfs.Xf[s].NumFmtID]
+	ok := builtInNumFmtFunc[*styleSheet.CellXfs.Xf[s].NumFmtID]
 	if ok != nil {
-		return ok(styleSheet.CellXfs.Xf[s].NumFmtID, v)
+		return ok(*styleSheet.CellXfs.Xf[s].NumFmtID, v)
 	}
 	return v
 }

go.mod

@@ -6,6 +6,7 @@ require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826
 	github.com/stretchr/testify v1.3.0
+	github.com/xuri/efp v0.0.0-20191019043341-b7dc4fe9aa91
 	golang.org/x/image v0.0.0-20190910094157-69e4b8554b2a
 	golang.org/x/net v0.0.0-20191209160850-c0dbc17a3553
 	golang.org/x/text v0.3.2 // indirect

go.sum

@@ -9,6 +9,8 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
+github.com/xuri/efp v0.0.0-20191019043341-b7dc4fe9aa91 h1:gp02YctZuIPTk0t7qI+wvg3VQwTPyNmSGG6ZqOsjSL8=
+github.com/xuri/efp v0.0.0-20191019043341-b7dc4fe9aa91/go.mod h1:uBiSUepVYMhGTfDeBKKasV4GpgBlzJ46gXUBAqV8qLk=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/image v0.0.0-20190910094157-69e4b8554b2a h1:gHevYm0pO4QUbwy8Dmdr01R5r1BuKtfYqRqF0h/Cbh0=
 golang.org/x/image v0.0.0-20190910094157-69e4b8554b2a/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=

styles.go

@@ -2299,21 +2299,21 @@ func setBorders(style *Style) *xlsxBorder {
 // cell.
 func setCellXfs(style *xlsxStyleSheet, fontID, numFmtID, fillID, borderID int, applyAlignment, applyProtection bool, alignment *xlsxAlignment, protection *xlsxProtection) int {
 	var xf xlsxXf
-	xf.FontID = fontID
+	xf.FontID = intPtr(fontID)
 	if fontID != 0 {
-		xf.ApplyFont = true
+		xf.ApplyFont = boolPtr(true)
 	}
-	xf.NumFmtID = numFmtID
+	xf.NumFmtID = intPtr(numFmtID)
 	if numFmtID != 0 {
-		xf.ApplyNumberFormat = true
+		xf.ApplyNumberFormat = boolPtr(true)
 	}
-	xf.FillID = fillID
+	xf.FillID = intPtr(fillID)
-	xf.BorderID = borderID
+	xf.BorderID = intPtr(borderID)
 	style.CellXfs.Count++
 	xf.Alignment = alignment
-	xf.ApplyAlignment = applyAlignment
+	xf.ApplyAlignment = boolPtr(applyAlignment)
 	if applyProtection {
-		xf.ApplyProtection = applyProtection
+		xf.ApplyProtection = boolPtr(applyProtection)
 		xf.Protection = protection
 	}
 	xfID := 0

styles_test.go

@@ -33,9 +33,9 @@ func TestStyleFill(t *testing.T) {
 		styles := xl.stylesReader()
 		style := styles.CellXfs.Xf[styleID]
 		if testCase.expectFill {
-			assert.NotEqual(t, style.FillID, 0, testCase.label)
+			assert.NotEqual(t, *style.FillID, 0, testCase.label)
 		} else {
-			assert.Equal(t, style.FillID, 0, testCase.label)
+			assert.Equal(t, *style.FillID, 0, testCase.label)
 		}
 	}
 }
@@ -188,7 +188,7 @@ func TestNewStyle(t *testing.T) {
 	assert.NoError(t, err)
 	styles := f.stylesReader()
 	fontID := styles.CellXfs.Xf[styleID].FontID
-	font := styles.Fonts.Font[fontID]
+	font := styles.Fonts.Font[*fontID]
 	assert.Contains(t, *font.Name.Val, "Times New Roman", "Stored font should contain font name")
 	assert.Equal(t, 2, styles.CellXfs.Count, "Should have 2 styles")
 	_, err = f.NewStyle(&Style{})

xmlStyles.go

@@ -209,19 +209,19 @@ type xlsxCellStyleXfs struct {
 // xlsxXf directly maps the xf element. A single xf element describes all of the
 // formatting for a cell.
 type xlsxXf struct {
-	NumFmtID          int             `xml:"numFmtId,attr,omitempty"`
+	NumFmtID          *int            `xml:"numFmtId,attr"`
-	FontID            int             `xml:"fontId,attr,omitempty"`
+	FontID            *int            `xml:"fontId,attr"`
-	FillID            int             `xml:"fillId,attr,omitempty"`
+	FillID            *int            `xml:"fillId,attr"`
-	BorderID          int             `xml:"borderId,attr,omitempty"`
+	BorderID          *int            `xml:"borderId,attr"`
-	XfID              *int            `xml:"xfId,attr,omitempty"`
+	XfID              *int            `xml:"xfId,attr"`
-	QuotePrefix       bool            `xml:"quotePrefix,attr,omitempty"`
+	QuotePrefix       *bool           `xml:"quotePrefix,attr"`
-	PivotButton       bool            `xml:"pivotButton,attr,omitempty"`
+	PivotButton       *bool           `xml:"pivotButton,attr"`
-	ApplyNumberFormat bool            `xml:"applyNumberFormat,attr,omitempty"`
+	ApplyNumberFormat *bool           `xml:"applyNumberFormat,attr"`
-	ApplyFont         bool            `xml:"applyFont,attr,omitempty"`
+	ApplyFont         *bool           `xml:"applyFont,attr"`
-	ApplyFill         bool            `xml:"applyFill,attr,omitempty"`
+	ApplyFill         *bool           `xml:"applyFill,attr"`
-	ApplyBorder       bool            `xml:"applyBorder,attr,omitempty"`
+	ApplyBorder       *bool           `xml:"applyBorder,attr"`
-	ApplyAlignment    bool            `xml:"applyAlignment,attr,omitempty"`
+	ApplyAlignment    *bool           `xml:"applyAlignment,attr"`
-	ApplyProtection   bool            `xml:"applyProtection,attr,omitempty"`
+	ApplyProtection   *bool           `xml:"applyProtection,attr"`
 	Alignment         *xlsxAlignment  `xml:"alignment"`
 	Protection        *xlsxProtection `xml:"protection"`
 }