Skip to content

Conversation

@taynaps
Copy link
Contributor

@taynaps taynaps commented Nov 14, 2016

#313 sync transactions merged by file path and name

return false;
}

private int countAllFiles(Boolean allFiles) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inverse of the parameter would be more readable imho: finishedFiles

filesList.add(transactionsList.get(0));
for (int i = 1; i < transactionsList.size(); i++) {
for (int j = 0; j < filesList.size(); j++) {
if (transactionsList.get(i).getDestination().equals(filesList.get(j).getDestination()) &&
Copy link
Member

@audax audax Nov 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Iterate with a foreach instead.
Also, this probably is a major performance issue since the algorithm is O(n²). A faster algorithm would be for example:

M is a map
foreach transaction:
  if (destination, filename) in map: skip
  else:  M[(destination, filename)] = transaction
return M.size

This uses a pair of destination and filename to filter out duplicates and is O(n). This hash could also be updated outside of the countAllFiles method, whenever a transaction is removed or added. To only count completed transactions, the implementation would be:
countAllFiles().filter{ it.isDone() }.size

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants