This optimize the code, simplify unit test and drawing object position calculation (#1561)

Co-authored-by: Nxinyao.cheng <xinyao.cheng@zerone.com.cn>

col.go

@@ -604,20 +604,21 @@ func flatCols(col xlsxCol, cols []xlsxCol, replacer func(fc, c xlsxCol) xlsxCol)
 //	width           # Width of object frame.
 //	height          # Height of object frame.
 func (f *File) positionObjectPixels(sheet string, col, row, x1, y1, width, height int) (int, int, int, int, int, int) {
+	colIdx, rowIdx := col-1, row-1
 	// Adjust start column for offsets that are greater than the col width.
-	for x1 >= f.getColWidth(sheet, col+1) {
-		x1 -= f.getColWidth(sheet, col+1)
-		col++
+	for x1 >= f.getColWidth(sheet, colIdx+1) {
+		colIdx++
+		x1 -= f.getColWidth(sheet, colIdx)
 	}
 
 	// Adjust start row for offsets that are greater than the row height.
-	for y1 >= f.getRowHeight(sheet, row+1) {
-		y1 -= f.getRowHeight(sheet, row+1)
-		row++
+	for y1 >= f.getRowHeight(sheet, rowIdx+1) {
+		rowIdx++
+		y1 -= f.getRowHeight(sheet, rowIdx)
 	}
 
 	// Initialized end cell to the same as the start cell.
-	colEnd, rowEnd := col, row
+	colEnd, rowEnd := colIdx, rowIdx
 
 	width += x1
 	height += y1
@@ -635,9 +636,7 @@ func (f *File) positionObjectPixels(sheet string, col, row, x1, y1, width, heigh
 	}
 
 	// The end vertices are whatever is left from the width and height.
-	x2 := width
-	y2 := height
-	return col, row, colEnd, rowEnd, x2, y2
+	return colIdx, rowIdx, colEnd, rowEnd, width, height
 }
 
 // getColWidth provides a function to get column width in pixels by given

drawing.go

@@ -1297,12 +1297,9 @@ func (f *File) addDrawingChart(sheet, drawingXML, cell string, width, height, rI
 	if err != nil {
 		return err
 	}
-	colIdx := col - 1
-	rowIdx := row - 1
-
 	width = int(float64(width) * opts.ScaleX)
 	height = int(float64(height) * opts.ScaleY)
-	colStart, rowStart, colEnd, rowEnd, x2, y2 := f.positionObjectPixels(sheet, colIdx, rowIdx, opts.OffsetX, opts.OffsetY, width, height)
+	colStart, rowStart, colEnd, rowEnd, x2, y2 := f.positionObjectPixels(sheet, col, row, opts.OffsetX, opts.OffsetY, width, height)
 	content, cNvPrID, err := f.drawingParser(drawingXML)
 	if err != nil {
 		return err

merge_test.go

@@ -13,14 +13,18 @@ func TestMergeCell(t *testing.T) {
 		t.FailNow()
 	}
 	assert.EqualError(t, f.MergeCell("Sheet1", "A", "B"), newCellNameToCoordinatesError("A", newInvalidCellNameError("A")).Error())
-	assert.NoError(t, f.MergeCell("Sheet1", "D9", "D9"))
-	assert.NoError(t, f.MergeCell("Sheet1", "D9", "E9"))
-	assert.NoError(t, f.MergeCell("Sheet1", "H14", "G13"))
-	assert.NoError(t, f.MergeCell("Sheet1", "C9", "D8"))
-	assert.NoError(t, f.MergeCell("Sheet1", "F11", "G13"))
-	assert.NoError(t, f.MergeCell("Sheet1", "H7", "B15"))
-	assert.NoError(t, f.MergeCell("Sheet1", "D11", "F13"))
-	assert.NoError(t, f.MergeCell("Sheet1", "G10", "K12"))
+	for _, cells := range [][]string{
+		{"D9", "D9"},
+		{"D9", "E9"},
+		{"H14", "G13"},
+		{"C9", "D8"},
+		{"F11", "G13"},
+		{"H7", "B15"},
+		{"D11", "F13"},
+		{"G10", "K12"},
+	} {
+		assert.NoError(t, f.MergeCell("Sheet1", cells[0], cells[1]))
+	}
 	assert.NoError(t, f.SetCellValue("Sheet1", "G11", "set value in merged cell"))
 	assert.NoError(t, f.SetCellInt("Sheet1", "H11", 100))
 	assert.NoError(t, f.SetCellValue("Sheet1", "I11", 0.5))
@@ -39,32 +43,29 @@ func TestMergeCell(t *testing.T) {
 
 	_, err = f.NewSheet("Sheet3")
 	assert.NoError(t, err)
-	assert.NoError(t, f.MergeCell("Sheet3", "D11", "F13"))
-	assert.NoError(t, f.MergeCell("Sheet3", "G10", "K12"))
-
-	assert.NoError(t, f.MergeCell("Sheet3", "B1", "D5")) // B1:D5
-	assert.NoError(t, f.MergeCell("Sheet3", "E1", "F5")) // E1:F5
-
-	assert.NoError(t, f.MergeCell("Sheet3", "H2", "I5"))
-	assert.NoError(t, f.MergeCell("Sheet3", "I4", "J6")) // H2:J6
-
-	assert.NoError(t, f.MergeCell("Sheet3", "M2", "N5"))
-	assert.NoError(t, f.MergeCell("Sheet3", "L4", "M6")) // L2:N6
-
-	assert.NoError(t, f.MergeCell("Sheet3", "P4", "Q7"))
-	assert.NoError(t, f.MergeCell("Sheet3", "O2", "P5")) // O2:Q7
 
-	assert.NoError(t, f.MergeCell("Sheet3", "A9", "B12"))
-	assert.NoError(t, f.MergeCell("Sheet3", "B7", "C9")) // A7:C12
-
-	assert.NoError(t, f.MergeCell("Sheet3", "E9", "F10"))
-	assert.NoError(t, f.MergeCell("Sheet3", "D8", "G12"))
-
-	assert.NoError(t, f.MergeCell("Sheet3", "I8", "I12"))
-	assert.NoError(t, f.MergeCell("Sheet3", "I10", "K10"))
-
-	assert.NoError(t, f.MergeCell("Sheet3", "M8", "Q13"))
-	assert.NoError(t, f.MergeCell("Sheet3", "N10", "O11"))
+	for _, cells := range [][]string{
+		{"D11", "F13"},
+		{"G10", "K12"},
+		{"B1", "D5"}, // B1:D5
+		{"E1", "F5"}, // E1:F5
+		{"H2", "I5"},
+		{"I4", "J6"}, // H2:J6
+		{"M2", "N5"},
+		{"L4", "M6"}, // L2:N6
+		{"P4", "Q7"},
+		{"O2", "P5"}, // O2:Q7
+		{"A9", "B12"},
+		{"B7", "C9"}, // A7:C12
+		{"E9", "F10"},
+		{"D8", "G12"},
+		{"I8", "I12"},
+		{"I10", "K10"},
+		{"M8", "Q13"},
+		{"N10", "O11"},
+	} {
+		assert.NoError(t, f.MergeCell("Sheet3", cells[0], cells[1]))
+	}
 
 	// Test merge cells on not exists worksheet
 	assert.EqualError(t, f.MergeCell("SheetN", "N10", "O11"), "sheet SheetN does not exist")

picture.go

@@ -332,16 +332,13 @@ func (f *File) addDrawingPicture(sheet, drawingXML, cell, ext string, rID, hyper
 	}
 	width, height := img.Width, img.Height
 	if opts.AutoFit {
-		width, height, col, row, err = f.drawingResize(sheet, cell, float64(width), float64(height), opts)
-		if err != nil {
+		if width, height, col, row, err = f.drawingResize(sheet, cell, float64(width), float64(height), opts); err != nil {
 			return err
 		}
 	} else {
 		width = int(float64(width) * opts.ScaleX)
 		height = int(float64(height) * opts.ScaleY)
 	}
-	col--
-	row--
 	colStart, rowStart, colEnd, rowEnd, x2, y2 := f.positionObjectPixels(sheet, col, row, opts.OffsetX, opts.OffsetY, width, height)
 	content, cNvPrID, err := f.drawingParser(drawingXML)
 	if err != nil {

picture_test.go

@@ -62,10 +62,9 @@ func TestAddPicture(t *testing.T) {
 	// Test add picture to worksheet from bytes with illegal cell reference
 	assert.EqualError(t, f.AddPictureFromBytes("Sheet1", "A", &Picture{Extension: ".png", File: file, Format: &GraphicOptions{AltText: "Excel Logo"}}), newCellNameToCoordinatesError("A", newInvalidCellNameError("A")).Error())
 
-	assert.NoError(t, f.AddPicture("Sheet1", "Q8", filepath.Join("test", "images", "excel.gif"), nil))
-	assert.NoError(t, f.AddPicture("Sheet1", "Q15", filepath.Join("test", "images", "excel.jpg"), nil))
-	assert.NoError(t, f.AddPicture("Sheet1", "Q22", filepath.Join("test", "images", "excel.tif"), nil))
-	assert.NoError(t, f.AddPicture("Sheet1", "Q28", filepath.Join("test", "images", "excel.bmp"), nil))
+	for cell, ext := range map[string]string{"Q8": "gif", "Q15": "jpg", "Q22": "tif", "Q28": "bmp"} {
+		assert.NoError(t, f.AddPicture("Sheet1", cell, filepath.Join("test", "images", fmt.Sprintf("excel.%s", ext)), nil))
+	}
 
 	// Test write file to given path
 	assert.NoError(t, f.SaveAs(filepath.Join("test", "TestAddPicture1.xlsx")))
@@ -99,15 +98,10 @@ func TestAddPictureErrors(t *testing.T) {
 	// Test add picture with custom image decoder and encoder
 	decode := func(r io.Reader) (image.Image, error) { return nil, nil }
 	decodeConfig := func(r io.Reader) (image.Config, error) { return image.Config{Height: 100, Width: 90}, nil }
-	image.RegisterFormat("emf", "", decode, decodeConfig)
-	image.RegisterFormat("wmf", "", decode, decodeConfig)
-	image.RegisterFormat("emz", "", decode, decodeConfig)
-	image.RegisterFormat("wmz", "", decode, decodeConfig)
-	image.RegisterFormat("svg", "", decode, decodeConfig)
-	assert.NoError(t, f.AddPicture("Sheet1", "Q1", filepath.Join("test", "images", "excel.emf"), nil))
-	assert.NoError(t, f.AddPicture("Sheet1", "Q7", filepath.Join("test", "images", "excel.wmf"), nil))
-	assert.NoError(t, f.AddPicture("Sheet1", "Q13", filepath.Join("test", "images", "excel.emz"), nil))
-	assert.NoError(t, f.AddPicture("Sheet1", "Q19", filepath.Join("test", "images", "excel.wmz"), nil))
+	for cell, ext := range map[string]string{"Q1": "emf", "Q7": "wmf", "Q13": "emz", "Q19": "wmz"} {
+		image.RegisterFormat(ext, "", decode, decodeConfig)
+		assert.NoError(t, f.AddPicture("Sheet1", cell, filepath.Join("test", "images", fmt.Sprintf("excel.%s", ext)), nil))
+	}
 	assert.NoError(t, f.AddPicture("Sheet1", "Q25", "excelize.svg", &GraphicOptions{ScaleX: 2.8}))
 	assert.NoError(t, f.SaveAs(filepath.Join("test", "TestAddPicture2.xlsx")))
 	assert.NoError(t, f.Close())

shape.go

@@ -326,13 +326,10 @@ func (f *File) addDrawingShape(sheet, drawingXML, cell string, opts *Shape) erro
 	if err != nil {
 		return err
 	}
-	colIdx := fromCol - 1
-	rowIdx := fromRow - 1
-
 	width := int(float64(opts.Width) * opts.Format.ScaleX)
 	height := int(float64(opts.Height) * opts.Format.ScaleY)
 
-	colStart, rowStart, colEnd, rowEnd, x2, y2 := f.positionObjectPixels(sheet, colIdx, rowIdx, opts.Format.OffsetX, opts.Format.OffsetY,
+	colStart, rowStart, colEnd, rowEnd, x2, y2 := f.positionObjectPixels(sheet, fromCol, fromRow, opts.Format.OffsetX, opts.Format.OffsetY,
 		width, height)
 	content, cNvPrID, err := f.drawingParser(drawingXML)
 	if err != nil {

stream_test.go

@@ -337,11 +337,9 @@ func TestStreamSetRowWithStyle(t *testing.T) {
 
 	ws, err := file.workSheetReader("Sheet1")
 	assert.NoError(t, err)
-	assert.Equal(t, grayStyleID, ws.SheetData.Row[0].C[0].S)
-	assert.Equal(t, zeroStyleID, ws.SheetData.Row[0].C[1].S)
-	assert.Equal(t, zeroStyleID, ws.SheetData.Row[0].C[2].S)
-	assert.Equal(t, blueStyleID, ws.SheetData.Row[0].C[3].S)
-	assert.Equal(t, blueStyleID, ws.SheetData.Row[0].C[4].S)
+	for colIdx, expected := range []int{grayStyleID, zeroStyleID, zeroStyleID, blueStyleID, blueStyleID} {
+		assert.Equal(t, expected, ws.SheetData.Row[0].C[colIdx].S)
+	}
 }
 
 func TestStreamSetCellValFunc(t *testing.T) {
@@ -352,25 +350,29 @@ func TestStreamSetCellValFunc(t *testing.T) {
 	sw, err := f.NewStreamWriter("Sheet1")
 	assert.NoError(t, err)
 	c := &xlsxC{}
-	assert.NoError(t, sw.setCellValFunc(c, 128))
-	assert.NoError(t, sw.setCellValFunc(c, int8(-128)))
-	assert.NoError(t, sw.setCellValFunc(c, int16(-32768)))
-	assert.NoError(t, sw.setCellValFunc(c, int32(-2147483648)))
-	assert.NoError(t, sw.setCellValFunc(c, int64(-9223372036854775808)))
-	assert.NoError(t, sw.setCellValFunc(c, uint(128)))
-	assert.NoError(t, sw.setCellValFunc(c, uint8(255)))
-	assert.NoError(t, sw.setCellValFunc(c, uint16(65535)))
-	assert.NoError(t, sw.setCellValFunc(c, uint32(4294967295)))
-	assert.NoError(t, sw.setCellValFunc(c, uint64(18446744073709551615)))
-	assert.NoError(t, sw.setCellValFunc(c, float32(100.1588)))
-	assert.NoError(t, sw.setCellValFunc(c, 100.1588))
-	assert.NoError(t, sw.setCellValFunc(c, " Hello"))
-	assert.NoError(t, sw.setCellValFunc(c, []byte(" Hello")))
-	assert.NoError(t, sw.setCellValFunc(c, time.Now().UTC()))
-	assert.NoError(t, sw.setCellValFunc(c, time.Duration(1e13)))
-	assert.NoError(t, sw.setCellValFunc(c, true))
-	assert.NoError(t, sw.setCellValFunc(c, nil))
-	assert.NoError(t, sw.setCellValFunc(c, complex64(5+10i)))
+	for _, val := range []interface{}{
+		128,
+		int8(-128),
+		int16(-32768),
+		int32(-2147483648),
+		int64(-9223372036854775808),
+		uint(128),
+		uint8(255),
+		uint16(65535),
+		uint32(4294967295),
+		uint64(18446744073709551615),
+		float32(100.1588),
+		100.1588,
+		" Hello",
+		[]byte(" Hello"),
+		time.Now().UTC(),
+		time.Duration(1e13),
+		true,
+		nil,
+		complex64(5 + 10i),
+	} {
+		assert.NoError(t, sw.setCellValFunc(c, val))
+	}
 }
 
 func TestStreamWriterOutlineLevel(t *testing.T) {
@@ -389,14 +391,10 @@ func TestStreamWriterOutlineLevel(t *testing.T) {
 
 	file, err = OpenFile(filepath.Join("test", "TestStreamWriterSetRowOutlineLevel.xlsx"))
 	assert.NoError(t, err)
-	level, err := file.GetRowOutlineLevel("Sheet1", 1)
-	assert.NoError(t, err)
-	assert.Equal(t, uint8(1), level)
-	level, err = file.GetRowOutlineLevel("Sheet1", 2)
-	assert.NoError(t, err)
-	assert.Equal(t, uint8(7), level)
-	level, err = file.GetRowOutlineLevel("Sheet1", 3)
-	assert.NoError(t, err)
-	assert.Equal(t, uint8(0), level)
+	for rowIdx, expected := range []uint8{1, 7, 0} {
+		level, err := file.GetRowOutlineLevel("Sheet1", rowIdx+1)
+		assert.NoError(t, err)
+		assert.Equal(t, expected, level)
+	}
 	assert.NoError(t, file.Close())
 }