You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by bu...@apache.org on 2015/08/15 09:15:19 UTC
[Bug 58245] New: [PATCH] Workbook interface should extend
Iterable
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
Bug ID: 58245
Summary: [PATCH] Workbook interface should extend
Iterable<Sheet>
Product: POI
Version: unspecified
Hardware: PC
OS: Linux
Status: NEW
Severity: normal
Priority: P2
Component: SS Common
Assignee: dev@poi.apache.org
Reporter: javenoneal@gmail.com
Created attachment 33001
--> https://bz.apache.org/bugzilla/attachment.cgi?id=33001&action=edit
patch to make Workbook interface implement Iterable<Sheet>
In one of my own projects, I would like to write code without specifying any
one particular implementation (HSSFWorkbook, XSSFWorkbook).
> Workbook wb = WorkbookFactory.create(file);
> for(Sheet worksheet : workbook) {
> // do something with worksheet
> }
Both HSSFWorkbook and XSSFWorkbook implement an iterator over the sheets, but
the Workbook interface is missing this, thus I can't write the above code
without defining the workbook as HSSF or XSSF.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58245] Make Workbook interface iterable over sheets
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
Javen ONeal <ja...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|Workbook interface should |Make Workbook interface
|extend Iterable<Sheet> |iterable over sheets
--- Comment #1 from Javen ONeal <ja...@gmail.com> ---
It looks like neither HSSFWorkbook nor XSSFWorkbook have sheet iterators
defined for them.
I'm not sure what's preferred in this case, upcasting all iterators to
Iterator<Sheet> or leaving the iterators in their more specific sheet type. The
former is how XSSFWorkbook.iterator is currently implemented. The latter is
similar to how XSSFSheet.rowIterator is currently implemented.
POI devs, what's preferred here?
============
ss.usermodel.Workbook:
public interface Workbook extends Closeable, Iterable {
Iterator<? extends Sheet> iterator();
}
xssf.usermodel.XSSFWorkbook:
public class XSSFWorkbook extends POIXMLDocument implements Workbook {
@Override
public Iterator<XSSFSheet> iterator() {
return sheets.iterator();
}
hssf.usermodel.HSSFWorkbook:
public final class HSSFWorkbook extends POIDocument implements Workbook {
@Override
public Iterator<HSSFSheet> iterator() {
return _sheets.iterator();
}
xssf.streaming.SXSSFWorkbook:
public class SXSSFWorkbook implements Workbook {
@Override
public Iterator<SXSSFSheet> iterator() {
return new SXSSFSheetIterator(_wb.iterator());
}
}
=================
ss.usermodel.Workbook:
public interface Workbook extends Closeable, Iterable {
Iterator<Sheet> iterator();
}
xssf.usermodel.XSSFWorkbook:
public class XSSFWorkbook extends POIXMLDocument implements Workbook {
@Override
public Iterator<Sheet> iterator() {
return (Iterator<Sheet>)(Iterator<? extends Sheet>) sheets.iterator();
}
hssf.usermodel.HSSFWorkbook:
public final class HSSFWorkbook extends POIDocument implements Workbook {
@Override
public Iterator<Sheet> iterator() {
return (Iterator<Sheet>)(Iterator<? extends Sheet>) _sheets.iterator();
}
xssf.streaming.SXSSFWorkbook:
public class SXSSFWorkbook implements Workbook {
@Override
public Iterator<Sheet> iterator() {
Iterator<SXSSFSheet> it = new SXSSFSheetIterator(_wb.iterator());
return (Iterator<Sheet>)(Iterator<? extends Sheet>) it;
}
}
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58245] Make Workbook interface iterable over sheets
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
Dominik Stadler <do...@gmx.at> changed:
What |Removed |Added
----------------------------------------------------------------------------
Severity|normal |enhancement
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58245] Make Workbook interface iterable over sheets
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
--- Comment #6 from Javen ONeal <ja...@gmail.com> ---
Created attachment 33067
--> https://bz.apache.org/bugzilla/attachment.cgi?id=33067&action=edit
site documentation updates
Updated quick-guide to include for-each sheet iteration in the attached patch.
These are the changes that developers would need to make with attachment 33066
final XSSFWorkbook wb = new XSSFWorkbook();
wb.createSheet();
// =====================================================================
// Case 1: Existing code uses XSSFSheet for-each loop
// =====================================================================
// Original code (no longer valid)
for (XSSFSheet sh : wb) {
sh.createRow(0);
}
// Option A:
for (XSSFSheet sh : (Iterable<XSSFSheet>) (Iterable<? extends Sheet>) wb) {
sh.createRow(0);
}
// Option B (preferred for new code):
for (Sheet sh : wb) {
sh.createRow(0);
}
// =====================================================================
// Case 2: Existing code creates an iterator variable
// =====================================================================
// Original code (no longer valid)
Iterator<XSSFSheet> it = wb.iterator();
XSSFSheet sh = it.next();
sh.createRow(0);
// Option A:
Iterator<XSSFSheet> it = (Iterator<XSSFSheet>) (Iterator<? extends Sheet>)
wb.iterator();
XSSFSheet sh = it.next();
sh.createRow(0);
// Option B (deprecated, but a quick-fix)
@SuppressWarnings("deprecation")
Iterator<XSSFSheet> it = wb.xssfSheetIterator();
XSSFSheet sh = it.next();
sh.createRow(0);
// Option C (preferred for new code):
Iterator<Sheet> it = wb.iterator();
Sheet sh = it.next();
sh.createRow(0);
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58245] [PATCH] Make Workbook interface iterable over sheets
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
Nick Burch <ap...@gagravarr.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
--- Comment #10 from Nick Burch <ap...@gagravarr.org> ---
Thanks for this, and for your patience!
Code change applied in r1703573, site in r1703574.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58245] [PATCH] Make Workbook interface iterable over sheets
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
--- Comment #8 from Nick Burch <ap...@gagravarr.org> ---
Looks good to me, thanks for this
I think that once we've got the build green again from the forbidden APIs check
changes, we should be fine to apply this as-is. Please remind us in 1-2 weeks
if we forget!
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58245] [PATCH] Make Workbook interface iterable over sheets
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
--- Comment #9 from Javen ONeal <ja...@gmail.com> ---
As of r1702321, build 841 <https://builds.apache.org/job/POI/841/>, the build
server is green again.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58245] Make Workbook interface iterable over sheets
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
--- Comment #2 from Andreas Beeker <ki...@apache.org> ---
I had the same intention with X/HSLF [1], but this ends in some interesting
generics definitions ... it works now, but I still think that some declarations
could be simpler ..
so I guess, I would do it as in org.apache.poi.sl.usermodel.ShapeContainer with
a parameterized interface
[1]
http://stackoverflow.com/questions/29440337/java-generics-parameterized-class-vs-typed-method
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58245] [PATCH] Make Workbook interface iterable over sheets
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
Javen ONeal <ja...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |PatchAvailable
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58245] Make Workbook interface iterable over sheets
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
Javen ONeal <ja...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #33001|0 |1
is obsolete| |
--- Comment #5 from Javen ONeal <ja...@gmail.com> ---
Created attachment 33066
--> https://bz.apache.org/bugzilla/attachment.cgi?id=33066&action=edit
patch to make all workbooks implement Iterable<Sheet>
The XSSFWorkbook.iterator breaks that pattern used by Sheet.iterator and
Row.iterator
Unfortunately, XSSFWorkbook has had this method for nearly 7 years (r700472
https://svn.apache.org/viewvc?view=revision&revision=700472).
Nonetheless, I think deprecating the old iterator is a step in the right
direction, allowing developers to write for loops like
for (Sheet sh : workbook) {
for (Row row : sh) {
for (Cell cell : row) {
System.out.println(cell);
}
}
}
I've provided test cases and documentation to help users transition their code
to the new Iterator<Sheet> iterator() interface (previously Iterator<XSSFSheet>
iterator().
Note: this patch will break backwards compatibility with existing code.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58245] Workbook interface should extend Iterable
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
Javen ONeal <ja...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|[PATCH] Workbook interface |Workbook interface should
|should extend |extend Iterable<Sheet>
|Iterable<Sheet> |
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58245] Make Workbook interface iterable over sheets
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
--- Comment #4 from Nick Burch <ap...@gagravarr.org> ---
I don't think we can do the Workbook<T> trick. I had a play with that quite
recently. The killer is that Eclipse (+friends) all start complaining when you
write something like "Workbook wb = WorkbookFactory.create(file)", claiming
that you should be giving it a generics type. We don't want to do that, as it
defeats the whole point of telling everyone to use the SS interfaces!
We certainly could do
Workbook: Iterator<? extends Sheet> getSheetIterator();
HSSFWorkbook: Iterator<HSSFSheet> getSheetIterator();
That works fine, and mirrors what we have for rows and cells. It's the iterable
version that I think will have to remain "Workbook implements Iterable<Sheet>"
and "HSSFWorkbook implements Workbook, Iterable<Sheet>", unless someone can
come up with a cunning workaround!
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58245] Make Workbook interface iterable over sheets
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
--- Comment #3 from Javen ONeal <ja...@gmail.com> ---
Mimicking what you wrote in r1692593:
org.apache.poi.sl.usermodel.ShapeContainer:
https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/sl/usermodel/ShapeContainer.java?limit_changes=0&r1=1692593&r2=1692592&pathrev=1692593
org.apache.poi.xslf.usermodel.XSLFShapeContainer:
https://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFShapeContainer.java?r1=1410315&r2=1692593&pathrev=1692593
Is this what you were referring to?
ss.usermodel.Workbook:
public interface Workbook<T extends Sheet> extends Closeable, Iterable<T> {
Iterator<T> iterator();
}
xssf.usermodel.XSSFWorkbook:
public class XSSFWorkbook extends POIXMLDocument implements Workbook<XSSFSheet>
{
@Override
public Iterator<XSSFSheet> iterator() {
return sheets.iterator();
}
hssf.usermodel.HSSFWorkbook:
public final class HSSFWorkbook extends POIDocument implements
Workbook<HSSFSheet> {
@Override
public Iterator<HSSFSheet> iterator() {
return _sheets.iterator();
}
xssf.streaming.SXSSFWorkbook:
public class SXSSFWorkbook implements Workbook<SXSSFSheet> {
@Override
public Iterator<SXSSFSheet> iterator() {
return new SXSSFSheetIterator(_wb.iterator());
}
}
I suppose I could go as far as writing a new interface:
public interface SheetContainer<T extends Sheet> extends Iterable<T>, but I
currently don't have any additional methods that a SheetContainer would need to
implement beyond what an Iterable requires.
This might be getting a bit out of my Java comfort zone here--I'm used to
Python where generics/templates aren't needed because everything is duck typed.
Suggestions?
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58245] [PATCH] Make Workbook interface iterable over sheets
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
Javen ONeal <ja...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|Make Workbook interface |[PATCH] Make Workbook
|iterable over sheets |interface iterable over
| |sheets
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58245] [PATCH] Make Workbook interface iterable over sheets
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58245
Javen ONeal <ja...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #33067|0 |1
is obsolete| |
--- Comment #7 from Javen ONeal <ja...@gmail.com> ---
Created attachment 33068
--> https://bz.apache.org/bugzilla/attachment.cgi?id=33068&action=edit
site documentation updates
fix quick-guide whitespace
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org