Replaced Vector with ArrayList, fixed Generics in Utils

DatabaseGroup / apted

APTED algorithm for the Tree Edit Distance

http://tree-edit-distance.dbresearch.uni-salzburg.at/

MIT License

115 stars 34 forks source link

Replaced Vector with ArrayList, fixed Generics in Utils #6

Closed miho closed 6 years ago

miho commented 6 years ago

Question: please let us know if you still maintain this codebase. If so, we are happy to send PRs. Here's a list of the most important fixes we addressed so far.

Since this project does not use multithreading the usage of Vector is not a good choice. Vector is fully synchronized and introduces some overhead. That's why it has been replaced by ArrayList.
Some utility methods were missing generics List vs List<String> etc. This has been corrected.
The current package names prevent APTED from being used as library since conflicts are very likely. There are too many util packages out there ;) We introduced package names derived from university & project name.
gradle project has been updated: it was missing encoding and Java version which often causes trouble. We also specify the gradle wrapper version which is important to prevent compatibility issues.

mateuszpawlik commented 6 years ago

Thanks for all the points. To finalise quickly, I've decided to make the changes myself. Here's the summary of what I'm planning to do. (1) I'll apply your changes to the Gradle part. However, I will not push the automatically generated files. For that I can offer to publish a tarball if it helps. I'll update Gradle version to the newest. I'll update the dependencies versions. (2) I'll change the packaging to avoid conflicts and add the prefix 'at.unisalzburg.dbresearch'. I'll leave the entry points of the old API and annotate as deprecated. That should simplify the transition for everyone using it. (3) I'll update README and javadoc respectively. (4) I'll change Vector->ArrayList and fix Generics. Does that sound reasonably?

miho commented 6 years ago

(1) I said above this is fine for us. We will provide the files in our fork. So there's no need for an additional archive. Such an archive would have to track changes between versions, e.g. Gradle 4 -> 5 etc. which is hard to achieve and makes thigs even more complex. (2) I would recommend to merge the Vector->ArrayList & Generic fixes as well. (3) Works for me.

mateuszpawlik commented 6 years ago

I've pushed a branch project-wide-change with all the changes we've discussed. As I've mentioned, it was easier for me this way due to the nature of changes. I'll merge with your approval.

miho commented 6 years ago

The changes look good to me. We did some early tests with you algorithm and so far it works great.

mateuszpawlik commented 6 years ago

I've pushed my changes to master. Can we close this pull request?

@miho Another question. Could you write me what exactly do you use our algorithm for?

miho commented 6 years ago

Thanks for adding the proposed changes to the master branch.