excel-streaming-reader
excel-streaming-reader copied to clipboard
pjfanning
# Refactoring & Improvement
- Created adapter for unsupported operations, this approach grant the concrete classes concern only on theirs busines logic.
- Created an enum for Qname instances. Enuns are singleton by default (see QNameBucket).
- Changed classes for to use the singleton instances of Qname.
- Changed utils or helpers classes to final by default.
- Created LazySupplier in a generic approach.
- Changed the classes to use the LazySupplier and removed old Supplier implementations.
- Extracted the duplicated code in a method on StreamingCell class. (see getCellTypeFromShortHandType).
- Moved some initializations to constructor in respective classes.
- Replaced "new Integer(20)" to "Integer.valueOf(20)" in Test class.
Thanks for your interest but I don't want this massive refactor. If you have specific changes, you can submit them in smaller chunks. Essentially, I don't want to change the class structure i ways that cause backward compatibility issues. I've only just released a 4.0.0 major release and don't want to do a 5.0.0 any time soon.
Okay, while it's a massive refactoring it doesn't necessarily break backwards compatibility. All interface contracts are maintained. And all unit tests are fully functional.
I've applied a couple of changes but the timing is bad - if this had come in a few days ago - prior to the 4.0.0 release, I would probably have just taken the whole PR but now I am reluctant to start moving around lots of code. Let me consider some of the refactors over the next few days.
Okay, if you need me to solve any questions, please tag my name in the comments here.
to be honest, I'm not sure the changes are needed - yes, it is tidier - but it makes binary compatibility changes that I do not want to add - to be frank, what this project lacks is test coverage - niceties about whether classes are final or not are not so important in the end of the day