Cleanup

[timrobertson100]

This project is a mess. There are comments about classes being removed in the near future and it is not clear how it is now meant to be used.

We should remove what can be and clean up. You get the sense it is overly complex for what should be a rather simple library

[cgendreau]

I'm happy to help. But I would be happier if the comment was more constructive.

[timrobertson100]

Fair point - and thanks! I was going to tackle this, but here is what I'd suggest - if sensible, we can create separate issues, but perhaps you know which classes can simply be removed to slim it down? (or marked as @deprecated with a comment to be removed in 1.33.

• All classes that are not meant to be used should be removed (e.g. those with "Warning: this class will probably be removed shortly"). This will remove the biggest amount of confusion.
• It is not clear why there are dwc and dwca packages. Especially when the dwc package contains things like Extensions which are only relevant for DwC Archives. Suggest refactor into something intuitive - perhaps only having packages of utils, xml, read, write or something along those lines
• The DarwinCoreRecord and DarwinCoreTaxon haven't been updated to follow the changes in the standard, so should probably be removed (they'll still exist in old versions of course)
• The API returns String for all values seemingly ignoring the delimitedBy. That should be documented if it is simply ignored (which as far as I can tell it is)
• ArchiveFactory is not deprecated but it is not clear when that should be used over DwcFiles. If not necessary and is to be removed then things like DownloadUtils can also be removed.
• StarRecordIterator has a comment with a TODO suggesting this can provide incorrect results if the comparator is not provided, but no issue exists for it - it's not clear whether that is something to be concerned with not. Suggest creating an issue and removing the comment as it causes confusion.
• All classes should have a Javadoc (e.g. NormalizedDwcArchive explaining what normalized means, IntSequenceGenerator which I suspect should be removed)

[cgendreau]

The "tabular" package was initially created as a solution for Issue #26 but was later replaced by what DwcFiles provides. The reason why it was still there once DwcFiles was created is simply that the validator was still based on it. After review we decided to also use the DwcFiles in the validator to keep consistency with the crawler and the poor package remained here, alone.

[cgendreau]

The initial reason why ArchiveFactory was not deprecated is that DwcFiles doesn't apply validation on the Archive (see validateArchive method in ArchiveFactory) so DwcFiles was not a direct replacement. But now, I would say it could be deprecated. What do you think @mdoering ?

[cgendreau]

It is not clear why there are dwc and dwca packages.

The goal (no trace/issue about it sorry) was to move everything in dwc and remove dwca. I would suggest keep the dwc package since an object from org.gbif.read.Archive is not really clear but I would definitely rework the packages to remove io and dwca.

[mdoering]

Supporting all the refactoring ideas. Just checked and my code still uses ArchiveFactory everywhere. Would be good to have just one factory, whichever it is I don't care. And validation could be a separate thing - if its needed at all

[timrobertson100]

@mdoering - would you prefer that we incorporate the refactor so that the ArchiveFactory API remains? I think that would be doable and might make adoption easier.

[cgendreau]

you mean to keep ArchiveFactory or to move its functionalities to DwcFiles?

[timrobertson100]

Was asking if he'd prefer to keep ArchiveFactory public methods replacing their functionality with that of the DwcFiles. I expect a lot of codebases might be using it.

[cgendreau]

ok, but it's already like that see https://github.com/gbif/dwca-io/blob/master/src/main/java/org/gbif/dwca/io/ArchiveFactory.java#L64

Unless I misunderstood.

[mdoering]

I reckon its simpler to keep ArchiveFactory instead of DwcFiles as its used a lot already and the main entry into the library. But its not a big deal for my codebase to switch to DwcFiles - whatever you guys prefer. But I also would like to see just one factory

[timrobertson100]

Thanks Markus, I suggest we keep the ArchiveFactory adding any new public methods necessary - mainly because we just don't know how many codebases will be affected.

…

On Sun, Mar 4, 2018 at 1:37 PM, Markus Döring ***@***.***> wrote: I reckon its simpler to keep ArchiveFactory instead of DwcFiles as its used a lot already and the main entry into the library. But its not a big deal for my codebase to switch to DwcFiles - whatever you guys prefer. But I also would like to see just one factory — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#42 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOepYHlXC82dCXsOC4XXY18frtRh8tiks5ta-ACgaJpZM4SXJRw> .

[cgendreau]

I would suggest to keep DwcFiles and deprecate ArchiveFactory (otherwise I would have not create DwcFiles initially).

Considering ArchiveDwcIterator and ArchiveIterator were deprecated in 1.32 deprecating ArchiveFactory seems consistent to me. We are talking around 20 instances to update outside this project it self.