Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why do we need checkRow function? #2017

Open
huantt opened this issue Oct 31, 2024 · 4 comments
Open

Why do we need checkRow function? #2017

huantt opened this issue Oct 31, 2024 · 4 comments

Comments

@huantt
Copy link

huantt commented Oct 31, 2024

Description

This function consumes too much memory, and I don't see the benefit of filling gap columns for rows.

Let's examine the GetColumnCell function:

for colIdx := range rowData.C {
			colData := &rowData.C[colIdx]
			if cell != colData.R {
				continue
			}
			val, ok, err := fn(ws, colData)
			if err != nil {
				return "", err
			}
			if ok {
				return val, nil
			}
		}

You are looping through the columns and checking the cell name to return a value.
Therefore, what is the purpose of filling gap columns here?

@huantt
Copy link
Author

huantt commented Nov 1, 2024

image

Oh, we will encounter a problem when writing to the cell.
The prepareCell function will return the wrong cell.
Can we update the logic of the prepareCell function to use a loop to find the cell instead of accessing it directly by index?

@xuri
Copy link
Member

xuri commented Nov 1, 2024

The library pre-allocates some consecutive cells for set cell value randomly. Please reference the documentation to using stream mode API (stream writer for generate worksheet, and rows iterator for reading worksheet) to reduce memory usage.

@huantt
Copy link
Author

huantt commented Nov 1, 2024

I tried using a Stream Reader (rows) and StreamW riter, but since the Stream Reader only returns an array of string values (without cell style), I cannot use them to maintain the style when writing.

@xuri
Copy link
Member

xuri commented Nov 1, 2024

There are no plan to change for the prepareCell recently, I dont think that use a loop to find the cell instead of accessing it directly by index can be reduce memory usage without much more time cost, if you'd like to create a pull request for this to improvement performance, that's would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants