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

Refactoring and code cleanup for XSSFSheet, XSSFRowColShifter, RowShifter and ColumnHelper classes #1066

Merged
merged 18 commits into from
Apr 26, 2023

Conversation

artem-iron
Copy link
Contributor

This PR aims to mainly cleanup and make more consistent and readable the code of XSSFSheet class,
but touching some adjacent classes, such as XSSFRowColShifter, RowShifter and ColumnHelper.

The core reason for this change to exist is to be a platform to later build functionality to manage columns of the sheet
the same way rows are managed, with column shifting etc. Hopefully this improves maintainability and stability somewhat. It is easier to track changes of XSSFSheet, XSSFRowColShifter, RowShifter and ColumnHelper classes in the commit history, since there's a lot of moving stuff around and diffs don't do a good job representing the picture of what happened. Public API has no breaking changes, no new failed tests are generated.

Among the changes:

  • Converts javadoc to csharp xml documentation in XSSFSheet, XSSFRowColShifter, RowShifter and ColumnHelper classes.
  • Adds CT_Cols.RemoveCols(ILst<CT_Col> method.
  • Adds CT_Comment.Set(CT_Comment) and CT_Shape.Set(CT_Shape) methods. The methods copy to
    current object properties of objects passed as parameters.
  • Adds XSSFRow.RebuildCells internal method. It sorts cells in low level row object and in the
    _cells field of XSSFRow. Order of the cells is imortant, Excel will repair the document if cells are out of order.
  • Adds ISheet.CopyComment (ICell, ICell) method to be used in SheetUtil.
  • Adds CT_Col.SetNumber(uint) method. It sets min and max properties of CT_Col that are used by
    excel as a span of a column object.
  • Adds CT_Col.IsSetNumber method. It indicates if min & max fields of CT_Col are more
    than 0. This means that CT_Col object has a place on the worksheet.
  • Adds null checks for CT_Col.colField. Initialize it if it's null.
  • Adds CT_Col.Set(CT_Col) method.
  • Adds CT_Col.UnsetWidth method.
  • Adds CT_Col.UnsetCustomWidth method.
  • Adds CT_Col.UnsetStyle method.
  • Breaks CellUtil.CopyCell method into two methods, to be later able to add an overload that
    accepts IColumn as argument.
  • Makes CT_Col.IsSetStyle return false if styleField equals zero. The style is not set if it is zero.
  • Removes unused parameter from SetColumn method of XSSFSheet. The method is private to XSSFSheet,
    not a breaking change.
  • Renames XSSFSheet.ShiftedRowNum private method to XSSFSheet.ShiftedRowOrColumnNumber,
    renames parameters to not be tied to only rows.
  • Renames XSSFSheet.ShouldRemoveRow private method to XSSFSheet.ShouldRemoveAtIndex,
    renames parapmeters to not be tied to rows.
  • Renames XSSFSheet.RemoveOverwritten private method to XSSFSheet.RemoveOverwrittenRows
  • Refactors XSSFSheet.HeadMapCount method to be more general, to be later able to use it fot columns.
  • XSSFRow now applies its style to all new cells created in it if it has style set to it. In
    accordance with excel's behavior.
  • Fixes possible null reference in XSSFCell.CopyCellFrom method. It could occur when hyperlinks were
    copied if policy dictates copying of hyperlinks, but the source cell is null.
  • Fixes AutoSizeRow tests.

Artem Koloskov added 18 commits April 19, 2023 10:57
Convert javadoc to csharp xml documentation. use 'var' istead of explicit types where possible.
Convert Javadoc to c# xml docs;
Add omitted cutly brackets;
clean duoble blank-lines;
add readonly modificators where appropriate;
use type matching where approptiate;
add omitted accessibility modificators;
remove unnecessary parenthesis where appropriate;
use simpified types (bool, string instead of Boolean, String);
Adjust code-line lengths to fit smaller editor windows
To be later able to add an overload that accepts IColumn as argument.
The methods copy to current object properties of objects passed as parameters.
Adjust code-line lengths to fit smaller editor windows in XSSFSheet
Convert javadoc to xml documentation for XSSFSheet
Remove redundant code from Write method of XSSFSheet

Remove unused parameter from SetColumn method of XSSFSheet
The method is private to XSSFSheet, not a breaking change

Simplify names in XSSFSheet
Add omitted cutly brackets;
clean duoble blank-lines;
add readonly modificators where appropriate;
use type matching where approptiate;
add omitted accessibility modificators;
use null propagation where appropriate;
omit 'this' notation;
remove unnecessary parenthesis where appropriate;
use coalesce espressions where appropriate;
use simpified types (bool, string instead of Boolean, String);
No code changes, the code comparisson will show a mess of changes, but this literally ONLY moves methods and properties from a place to place.
Rename to XSSFSheet.ShiftedRowOrColumnNumber, rename parameters to not be tied to only rows.

Refactor XSSFSheet.ShouldRemoveRow private method

Rename to XSSFSheet.ShouldRemoveAtIndex, rename parapmeters to not be tied to rows.

Rename XSSFSheet.RemoveOverwritten private method to XSSFSheet.RemoveOverwrittenRows

Refactor XSSFSheet.HeadMapCount method to be more general.
To be later able to use it fot columns.
It sorts cells in low level row object and in the _cells field of XSSFRow. Order of the cells is imortant, Excel will repair the document if cells are out of order.
…s style set to it.

I accordance with excel's behavior
Use ISheet.CopyComment in SheetUtil
It could occur when hyperlinks are copied if policy dictates copying of hyperlinks, but the source cell is null.
Add CT_Col.SetNumber(uint) method.

It sets min and max properties of CT_Col that are used by excel as a span of a column object.

Add CT_Col.IsSetNumber method

It indicates if min & max fields of CT_Col are more than 0. This means that CT_Col object has a place on the worksheet.

Add null checks for CT_Col.colField

Initialize it if it's null

Add CT_Col.Set(CT_Col) method.

Add UnsetWidth, UnsetCustomWidth and UnsetStyle to CT_Col
No changes to code itself, just remove clatter, add braces where appropriate, move members to regions,
@artem-iron artem-iron changed the title Ref cl Refactoring and code cleanup for XSSFSheet, XSSFRowColShifter, RowShifter and ColumnHelper classes Apr 19, 2023
@tonyqus
Copy link
Member

tonyqus commented Apr 26, 2023

LGTM

@tonyqus tonyqus added this to the NPOI 2.6.1 milestone Apr 26, 2023
@tonyqus tonyqus merged commit 558fa81 into nissl-lab:master Apr 26, 2023
@artem-iron artem-iron deleted the ref-cl branch April 27, 2023 03:39
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

Successfully merging this pull request may close these issues.

None yet

2 participants