OpenPDF icon indicating copy to clipboard operation
OpenPDF copied to clipboard

Modernization efforts

Open rtfarte opened this issue 6 years ago • 16 comments

  • Add generics
  • Add try-with-resource options
  • Use foreach instead of iterators
  • Lambdas and streams?
  • Maintain backwards compatibility

rtfarte avatar Jan 16 '19 15:01 rtfarte

I also would like to have the library updated to be type safe and use all the "modern" features of Java 8. I volunteer to update the project to use the aforementioned features, if needed.

This will very likely break the current interface, because lots of non-typed parameters in the interface are exposed in the public API. Is there a specific branch I should start from? (Maybe a 1.4.x version)

fvbassi avatar Jan 23 '19 12:01 fvbassi

Pull-requests to modernize OpenPDF are welcome!

This will very likely break the current interface

We can not break the current interface (API) of OpenPDF. The project currently has a goal to be compatible with iText 2.x and 4.x. We can add new classes, but not break the current API now.

andreasrosdal avatar Jan 23 '19 15:01 andreasrosdal

Do you think is it possible to change public methods from public ArrayList getChunks(); to public ArrayList<Chunk> getChunks(); ?

fvbassi avatar Jan 24 '19 08:01 fvbassi

@fvbassi Yes, I think changing public methods in ways so that it will still work for existing users should be possible. This doesn't break the API, I think, since the users of the getChunks() are expecting an ArrayList with Chunk objects, so the code calling the library will still compile and work.

Make sure that OpenPDF will still work for existing users. I think we need the opinions of many other OpenPDF developers and users of OpenPDF before making such changes, though. Pull-requests welcome!

andreasrosdal avatar Jan 24 '19 13:01 andreasrosdal

One possible solutions would be to create new, generic-enabled classes that extend the functionality. Another possibility could be to add new methods that accept generics.

I believe that returning generics on existing classes is acceptable. Is the only concern for methods that accept a generic list or updating a class to be generic?

I also think that adding try-with-resource should not affect any current clients of the library.

rtfarte avatar Jan 24 '19 16:01 rtfarte

I believe that returning generics on existing classes is acceptable

Let's start with this. Pull-requests are welcome!

I also think that adding try-with-resource should not affect any current clients of the library

Also a very good idea!

andreasrosdal avatar Jan 25 '19 04:01 andreasrosdal

Let me work on it. I'm facing some difficulties, but I'm still on track. Let me create a proposal. We will then discuss some details.

On Fri, Jan 25, 2019 at 5:09 AM Andreas Rosdal [email protected] wrote:

I believe that returning generics on existing classes is acceptable

Let's start with this. Pull-requests are welcome!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LibrePDF/OpenPDF/issues/139#issuecomment-457449063, or mute the thread https://github.com/notifications/unsubscribe-auth/AsybxREVO6BqG74Hu4DVooxSag85aw9Fks5vGoNdgaJpZM4aDXyK .

fvbassi avatar Jan 25 '19 08:01 fvbassi

@fvbassi I've seen that you have done a lot of good work in your own fork on the modernization of OpenPDF. Perhaps you could submit a pull-request soon, so we can include your changes?

andreasrosdal avatar Feb 15 '19 10:02 andreasrosdal

Let me have a bit more time to cleanup few details... I don't feel comfortable with a couple of things.

And I also want to do some testing.

–fb

Il giorno ven 15 feb 2019, 11:43 AM Andreas Rosdal [email protected] ha scritto:

@fvbassi https://github.com/fvbassi I've seen that you have done a lot of good work in your own fork on the modernization of OpenPDF. Perhaps you could submit a pull-request soon, so we can include your changes?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LibrePDF/OpenPDF/issues/139#issuecomment-463994023, or mute the thread https://github.com/notifications/unsubscribe-auth/AsybxU-MbZho5esbQZmtdksolqJz8EBVks5vNo9OgaJpZM4aDXyK .

fvbassi avatar Feb 15 '19 11:02 fvbassi

Sorry... You are right. Let me open a PR for these changes. I will fix other things in a subsequent one: unfortunately in these week I can't work on it. I will highlight in the PR a couple of problems that are hard to solve without breaking the API:

  • the class Phrase (and another one that I can't remember), used to extend ArrayList (as a contained of the children elements). This is mostly impossible to keep, because if we change the signature to be "class Phrase extends ArrayList<Element>", this will make some bridge method clash. So, I removed ArrayList<> as a superclass and added it as a field. But this may break the compatibility, due to the methods implicitely exported from the class ArrayList.

Let me now try to learn how to open my first pull request!

– fb

On Tue, Feb 26, 2019 at 7:26 AM Andreas Rosdal [email protected] wrote:

@fvbassi https://github.com/fvbassi Any news?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LibrePDF/OpenPDF/issues/139#issuecomment-467312356, or mute the thread https://github.com/notifications/unsubscribe-auth/AsybxakdeP0k2t6LIpSaZwrBR37WDqOMks5vRNOOgaJpZM4aDXyK .

fvbassi avatar Feb 26 '19 09:02 fvbassi

I should have already created one, last week. Let me check...

Il lun 4 mar 2019, 7:38 AM Andreas Rosdal [email protected] ha scritto:

@fvbassi https://github.com/fvbassi Any news?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LibrePDF/OpenPDF/issues/139#issuecomment-469136085, or mute the thread https://github.com/notifications/unsubscribe-auth/AsybxWBkpzbuZGf-Wn7RLQVy_jxTh0WHks5vTL-BgaJpZM4aDXyK .

fvbassi avatar Mar 04 '19 09:03 fvbassi

I did that against a wrong branch. Sorry. I recreated a new one. Sorry for the delay.

It's a big change. Feel free to merge it into a separate branch, if you want, for further tests.

On Mon, Mar 4, 2019 at 10:06 AM Francesco Bassi [email protected] wrote:

I should have already created one, last week. Let me check...

Il lun 4 mar 2019, 7:38 AM Andreas Rosdal [email protected] ha scritto:

@fvbassi https://github.com/fvbassi Any news?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LibrePDF/OpenPDF/issues/139#issuecomment-469136085, or mute the thread https://github.com/notifications/unsubscribe-auth/AsybxWBkpzbuZGf-Wn7RLQVy_jxTh0WHks5vTL-BgaJpZM4aDXyK .

fvbassi avatar Mar 04 '19 09:03 fvbassi

As part of the modernization efforts, the next version of OpenPDF will be 1.3.0. In this new version, we may modernize the API, so breaking API changes are allowed in order to utlilize new Java features.

@fvbassi @rtfarte Please help with this modernization effort if you can.

The API changes can be documented here: https://github.com/LibrePDF/OpenPDF/wiki/OpenPDF-1.3.0-API-changes

andreasrosdal avatar Jul 15 '19 06:07 andreasrosdal

@albfernandez @asturio @rtfarte @fvbassi @LibrePDF/developers

Your experience and assistance is needed to make this modernization a success. Some PRs have been merged now, breaking the API, and using new Java features. So, please help with this modernization.

andreasrosdal avatar Jul 15 '19 11:07 andreasrosdal

I've reviewed the pull request. I think we may try to be backwards compatible in source and binary form.

I think changing private, protected and package elements it's ok, but public elements should be taken with extreme care.

Some thoughs

Moving from raw ArrayList typed List<>. I think we must remain ArrayList<>. Example: public ArrayList getChunks() is changed to public java.util.List<Element> getChunks()

To be fully backwards compatible it should be: public ArrayList<Element> getChunks()

When changing parameters to other more specific, we can maintain the old method example public boolean add(Object o) -> public boolean add(Element o)

/** @ deprecated use add(Element)
@ Deprecated
public boolean add(Object o) {
   add((Element) o);
}
public boolean add (Element o) {
// ...
}

I think replacing itext with OpenPDF in projects which still use and compile using itext (jasperreports for me).

It's a idea to be discused. If you're ok I can provide a pull request.

albfernandez avatar Jul 22 '19 21:07 albfernandez

@albfernandez Your proposal is good. Thanks! Please provide a pull request.

andreasrosdal avatar Jul 24 '19 14:07 andreasrosdal