Abstract classes vs Util classes

[Javen O'Neal <ja...@gmail.com>]

We currently have several Util classes that serve the purpose of reducing
duplicated code between HSSF and XSSF.

I rambled about this all little on bug 58787 [1], but figured a mail list
was more appropriate for discussion.

There are certainly some good cases for having Util classes, but sometimes
the best thing is to have the functionality right in the class. The problem
is there is no place to put code that would be used by all implementing
classes besides a utility.

Perhaps we should convert Workbook, Sheet, Row, and Cell (and really
everything) to abstract classes so that generic code can be written in one
place rather than duplicated across implementing classes. I don't know if
this means we want to insert another level into the inheritance tree, such
as interface ICell -> abstract Cell -> implementing class HSSFCell. If we
don't want to separate interface from abstract class, we could make Cell
abstract, which wouldn't affect backwards compatibility.

This applies to Common SL as well, and is something to keep in mind for all
classes.

Thoughts?

[1] https://bz.apache.org/bugzilla/show_bug.cgi?id=58787#c8

Re: Abstract classes vs Util classes

[Andreas Beeker <ki...@apache.org>]

Hi Javen,

I've already thought about writing to the dev list when I've read your comments on bug 58787.
And now that you've mentioned common sl, I think I need to give my two pence ...

With common sl I also had to made that decision of going the *ssf way or having base classes (e.g. for drawing).
My problem with the base classes are the various aspects which needs to provided:
- there are format dependent base functions
- there is marshalling/binding code, drawing code, utility code (e.g. border functions)
- there are changes ... in case of SL, still a lot
- users want to customize, how POI works

/seperation of concerns:/
One of the first ideas was to split the drawing code from the implementation.
You can register your own Drawing classes at the DrawFactory without touching the binding
stuff. Although the Drawing classes currently have a similar composition hierarchy as the
binding classes, this wouldn't be necessary and different "boxing" could be possible.

/where to pass the buck:/
Of course I also have my problems with utility code, e.g. DrawTableShape.*Borders().
This is not used for drawing, but I didn't want to duplicate it in the binding code.
So I could either introduce another set of utility hierarchy code,
put it to the drawing classes, or introduce a common base class just for such logic.
As I'm looking at the format specific classes now as binding classes, this doesn't make sense
to put there ... I chose the 2nd option, but I'm also not really happy with that.

/light weighted binding classes/:
having that idea about replacing xmlbeans with jaxb someday, I thought its better to
have light weighted classes (... or getting them lighter) - when you start pulling the code into the base classes its
still part of the class and needs to be taken care of.

/interface vs. implementation/:
the format specific classes were in the beginning in two different jars (ooxml, scratchpad)
and haven't had a common interface. My problem was, how can I have a common base
which doesn't change so much (or at least just increases in functionality) without knowing
in which direction I would e-/merge?
Having an interface instead of a base classes gives me the freedom to say, these are the method
sets (e.g. PlaceableShape) which are more likely not to change, leave the internals to the implementation.
I can then decide at which inheritance level I want to provide a certain feature -
this is a bit different to base classes functionality, where common methods might need
to deal with corner cases or child classes overwrite that method or are forced to implement
abstract methods to help that common method.
(yes, there are still a lot of those examples in *SLF too ... I hope its getting better)

/alternative approach/:
I haven't played with Java 8 language features yet, but maybe "defaults" could be
a interesting concepts for Utility methods [1].
Not sure if the logic about pulling code into base functionality applies here ...

tl;dr: I don't want common base classes for x/hslf currently and I'd rather like
to separate binding classes, from user model and code working on the user model,
i.e. focusing the functionality - I don't want a "eierlegende wollmilchsau" ;)

Sorry Javen, I'll invite you one day to a free beer (at the apache con) or a free flight.

Andi.

PS: I wanted to add another few sentences about how tempting it is to have base classes
and to pull up code and find nice names for it [2], but that would be just another repetition ...


[1] http://stackoverflow.com/questions/7857832/are-defaults-in-jdk-8-a-form-of-multiple-inheritance-in-java
[2] http://blog.fefe.de/?ts=aea5fb5a

On 19.01.2016 00:08, Javen O'Neal wrote:
> We currently have several Util classes that serve the purpose of reducing
> duplicated code between HSSF and XSSF.
>
> I rambled about this all little on bug 58787 [1], but figured a mail list
> was more appropriate for discussion.
>
> There are certainly some good cases for having Util classes, but sometimes
> the best thing is to have the functionality right in the class. The problem
> is there is no place to put code that would be used by all implementing
> classes besides a utility.
>
> Perhaps we should convert Workbook, Sheet, Row, and Cell (and really
> everything) to abstract classes so that generic code can be written in one
> place rather than duplicated across implementing classes. I don't know if
> this means we want to insert another level into the inheritance tree, such
> as interface ICell -> abstract Cell -> implementing class HSSFCell. If we
> don't want to separate interface from abstract class, we could make Cell
> abstract, which wouldn't affect backwards compatibility.
>
> This applies to Common SL as well, and is something to keep in mind for all
> classes.
>
> Thoughts?
>
> [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=58787#c8
>

Re: Abstract classes vs Util classes

[Dominik Stadler <do...@gmx.at>]

Hi,

I'm also in favour of keeping the interfaces, for me it provides a very
concise and clear API for users, we should not clutter this with
implementation details.

One other possibility would be an intermediate abstract class which
collects these "common" code, i.e. AbstractWorkbook, AbstractSheet, ...
This should also be mostly backwards compatible.

Dominik.


On Tue, Jan 19, 2016 at 8:34 AM, Javen O'Neal <ja...@gmail.com> wrote:

> > Also, is there that much that's common for (eg) HSSFCell & XSSFCell &
> SXSSFCell & that proposed hybrid sax reading thingy?
> How about [1] XSSFSheet.copyCellFrom(Cell, CellCopyPolicy), which I
> haven't added to HSSFCell and the Cell interface yet partially because
> I didn't want to duplicate the code.
>
> Skimming through XSSFSheet and XSSFCell, I didn't see as much
> code as my memory had though that looked generic enough that it
> could be written once withoutvreferring to implementation classes.
> The methods that could benefit are mostly higher-level concepts
> like shiftRows and copyRows, though we might not be able to move
> these up if they rely on class-private data structures.
>
> For now it sounds like we're best off to leave the class hierarchy as
> is. When adding new code that is identical for HSSF and XSSF, say for
> example validateMergedRegions for bug 58885, should I move the
> would-be-duplicated code out to a *Helper (or *Util if helpful for
> external usage), or duplicate the code?
>
> Thanks for the discussion, folks!
>
> [1]
> http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java
>
> On Mon, Jan 18, 2016 at 9:29 PM, Nick Burch <ap...@gagravarr.org> wrote:
> > On Mon, 18 Jan 2016, Javen O'Neal wrote:
> >>
> >> There are certainly some good cases for having Util classes, but
> sometimes
> >> the best thing is to have the functionality right in the class. The
> problem
> >> is there is no place to put code that would be used by all implementing
> >> classes besides a utility.
> >
> >
> > Minor naming thing - on the whole, we use *Util classes for things that
> end
> > users will call, and *Helper for things that POI will internally call.
> Isn't
> > always consistent, but that's generally the plan!
> >
> >> Perhaps we should convert Workbook, Sheet, Row, and Cell (and really
> >> everything) to abstract classes so that generic code can be written in
> one
> >> place rather than duplicated across implementing classes.
> >
> >
> > That'd sadly break backwards compatibility for almost all of our users,
> even
> > if only at the binary level.
> >
> > In the past, there has been talk of other formats (eg ODF) implementing
> our
> > interfaces to get consistent handling for their spreadsheets too. I
> haven't
> > heard of any public implementations of that, but there may be private
> ones
> > that we'd risk breaking
> >
> > Also, is there that much that's common for (eg) HSSFCell & XSSFCell &
> > SXSSFCell & that proposed hybrid sax reading thingy? (Genuine question!)
> >
> > Nick
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> > For additional commands, e-mail: dev-help@poi.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> For additional commands, e-mail: dev-help@poi.apache.org
>
>

Re: Abstract classes vs Util classes

[Javen O'Neal <ja...@gmail.com>]

> Also, is there that much that's common for (eg) HSSFCell & XSSFCell & SXSSFCell & that proposed hybrid sax reading thingy?
How about [1] XSSFSheet.copyCellFrom(Cell, CellCopyPolicy), which I
haven't added to HSSFCell and the Cell interface yet partially because
I didn't want to duplicate the code.

Skimming through XSSFSheet and XSSFCell, I didn't see as much
code as my memory had though that looked generic enough that it
could be written once withoutvreferring to implementation classes.
The methods that could benefit are mostly higher-level concepts
like shiftRows and copyRows, though we might not be able to move
these up if they rely on class-private data structures.

For now it sounds like we're best off to leave the class hierarchy as
is. When adding new code that is identical for HSSF and XSSF, say for
example validateMergedRegions for bug 58885, should I move the
would-be-duplicated code out to a *Helper (or *Util if helpful for
external usage), or duplicate the code?

Thanks for the discussion, folks!

[1] http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java

On Mon, Jan 18, 2016 at 9:29 PM, Nick Burch <ap...@gagravarr.org> wrote:
> On Mon, 18 Jan 2016, Javen O'Neal wrote:
>>
>> There are certainly some good cases for having Util classes, but sometimes
>> the best thing is to have the functionality right in the class. The problem
>> is there is no place to put code that would be used by all implementing
>> classes besides a utility.
>
>
> Minor naming thing - on the whole, we use *Util classes for things that end
> users will call, and *Helper for things that POI will internally call. Isn't
> always consistent, but that's generally the plan!
>
>> Perhaps we should convert Workbook, Sheet, Row, and Cell (and really
>> everything) to abstract classes so that generic code can be written in one
>> place rather than duplicated across implementing classes.
>
>
> That'd sadly break backwards compatibility for almost all of our users, even
> if only at the binary level.
>
> In the past, there has been talk of other formats (eg ODF) implementing our
> interfaces to get consistent handling for their spreadsheets too. I haven't
> heard of any public implementations of that, but there may be private ones
> that we'd risk breaking
>
> Also, is there that much that's common for (eg) HSSFCell & XSSFCell &
> SXSSFCell & that proposed hybrid sax reading thingy? (Genuine question!)
>
> Nick
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> For additional commands, e-mail: dev-help@poi.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org

Re: Abstract classes vs Util classes

[Nick Burch <ap...@gagravarr.org>]

On Mon, 18 Jan 2016, Javen O'Neal wrote:
> There are certainly some good cases for having Util classes, but 
> sometimes the best thing is to have the functionality right in the 
> class. The problem is there is no place to put code that would be used 
> by all implementing classes besides a utility.

Minor naming thing - on the whole, we use *Util classes for things that 
end users will call, and *Helper for things that POI will internally call. 
Isn't always consistent, but that's generally the plan!

> Perhaps we should convert Workbook, Sheet, Row, and Cell (and really
> everything) to abstract classes so that generic code can be written in one
> place rather than duplicated across implementing classes.

That'd sadly break backwards compatibility for almost all of our users, 
even if only at the binary level.

In the past, there has been talk of other formats (eg ODF) implementing 
our interfaces to get consistent handling for their spreadsheets too. I 
haven't heard of any public implementations of that, but there may be 
private ones that we'd risk breaking

Also, is there that much that's common for (eg) HSSFCell & XSSFCell & 
SXSSFCell & that proposed hybrid sax reading thingy? (Genuine question!)

Nick

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org