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

open rows totalRow property #1054

Merged
merged 2 commits into from
Nov 4, 2021
Merged

open rows totalRow property #1054

merged 2 commits into from
Nov 4, 2021

Conversation

oneweek20169902
Copy link
Contributor

PR Details

Rows struct 开放私有属性totalRow

Description

Rows.Next() totalRow>TotalRow
Rows() totalRow>TotalRow

Motivation and Context

因为我需要将读出来的数据写入到数据库,如果单条写入,速度太慢,所以需要批量写入。而批量写入的拆分条件就需要根据总量计算出来。

How Has This Been Tested

Types of changes

  • [ x] Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • [ x] My code follows the code style of this project.
  • [ x] My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov-commenter
Copy link

codecov-commenter commented Nov 4, 2021

Codecov Report

Merging #1054 (d101532) into master (e64775f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1054   +/-   ##
=======================================
  Coverage   97.70%   97.70%           
=======================================
  Files          29       29           
  Lines       17011    17019    +8     
=======================================
+ Hits        16621    16629    +8     
  Misses        267      267           
  Partials      123      123           
Flag Coverage Δ
unittests 97.70% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
col.go 99.31% <100.00%> (+<0.01%) ⬆️
rows.go 94.58% <100.00%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e64775f...d101532. Read the comment docs.

@xuri xuri merged commit 60b13af into qax-os:master Nov 4, 2021
@xuri xuri added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 9, 2021
@xuri
Copy link
Member

xuri commented Jan 18, 2022

Hi @oneweek20169902, I've accepted this PR and introduced TotalCols, CurrentCol, TotalRows, and CurrentRow for the column and row's iterator in version v2.5.0. Since the performance optimization commit 4daa6ed, the internal variable totalRows was no longer available in the row's iterator, I refactor the row's iterator drop the pre-streamingly parsing for the worksheet for reducing the memory usage and time cost, so I made a breaking change: removed the exported function TotalRows. For consistent reason and following the minimum usable design principle, I decided to also remove another 3 functions: TotalCols, CurrentCol, CurrentRow, because developers could be easy to get total columns/rows by iterator without fetching every single column/row's cell value, and in the iterator progress, developers also could get the current column/row number by a simple counter, and the performance also similar with TotalCols/TotalRows in the same data scale. This breaking change will be released in the next version. What do you think about this, if you have any suggestions please let me know.

xuri added a commit that referenced this pull request Jan 18, 2022
This removed 3 exported functions: `TotalCols`, `CurrentCol` and `CurrentRow`
xuri added a commit to carbin-gun/excelize that referenced this pull request Oct 9, 2022
This removed 3 exported functions: `TotalCols`, `CurrentCol` and `CurrentRow`
jenbonzhang pushed a commit to jenbonzhang/excelize that referenced this pull request Oct 22, 2023
jenbonzhang pushed a commit to jenbonzhang/excelize that referenced this pull request Oct 22, 2023
This removed 3 exported functions: `TotalCols`, `CurrentCol` and `CurrentRow`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants