refactor: rename and deprecate legacy core classes #688
Conversation
delei
changed the title
| * Core classes of the Fesod spreadsheet processor | ||
| */ | ||
| public class FesodSheet extends FesodSheetFactory {} | ||
| public class FesodSheet { |
There was a problem hiding this comment.
Why remove FesodSheetFactory as that's from factory pattern?
There was a problem hiding this comment.
Ensure a single core class. In fact, these two types are not much different and they can also avoid some of the checks and prompts in the IDE.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the core class hierarchy by consolidating FesodSheetFactory into FesodSheet and updating deprecated alias classes to inherit from the new structure. The changes eliminate code duplication by removing the factory class and moving its functionality directly into FesodSheet, while maintaining backward compatibility through deprecated aliases.
Key Changes
- Removed
FesodSheetFactoryclass entirely and moved all its methods intoFesodSheet - Updated deprecated alias classes (
FastExcelFactory,FastExcel,EasyExcel) to extendFesodSheetinstead ofFesodSheetFactory - Enhanced deprecation documentation with clearer messages and migration guidance
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
FesodSheetFactory.java |
Removed the entire factory class (333 lines deleted) |
FesodSheet.java |
Changed from a simple alias extending FesodSheetFactory to a standalone class containing all factory methods previously in FesodSheetFactory |
FastExcelFactory.java |
Updated inheritance from FesodSheetFactory to FesodSheet and improved deprecation documentation |
FastExcel.java |
Updated inheritance from FesodSheetFactory to FesodSheet and improved deprecation documentation |
EasyExcel.java |
Updated inheritance from FesodSheetFactory to FesodSheet and improved deprecation documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Build excel the write | ||
| * | ||
| * @return |
There was a problem hiding this comment.
Missing @return description in JavaDoc. The @return tag should include a description of what is returned (e.g., "Excel writer builder").
| * @return | |
| * @return Excel writer builder |
| * @param sheetNo Index of sheet,0 base. | ||
| * @param sheetName The name of sheet. | ||
| * @param numRows The number of rows to read, the default is all, start with 0. | ||
| * @return |
There was a problem hiding this comment.
Missing @return description in JavaDoc. The @return tag should include a description of what is returned (e.g., "Excel sheet reader builder").
| * @return | |
| * @return Excel sheet reader builder. |
psxjoy
added
the
PR: require-multiple-approvals
psxjoy
left a comment
psxjoy
left a comment
There was a problem hiding this comment.
I don't have strong feelings about whether to keep FesodFactory.
Let's hear from the community: if there are no objections within 72 hours, this PR will be merged.
我对于是否保留FesodFactory并没有太多的坚持。
让我们听取一下社区的声音：如果 72 小时内没有人反对。该 PR 将合并。
3 participants
Purpose of the pull request
Close #651
What's changed?
Checklist