Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Convert from List to Set to speed up contains() logic. #2354

Merged
merged 2 commits into from
Aug 22, 2018
Merged

Convert from List to Set to speed up contains() logic. #2354

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
501b8c2
Convert from List to Set to speed up contains() logic.
Aug 8, 2018
0b0b37a
Just return a List.
Aug 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/main/java/spoon/support/compiler/VirtualFolder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,17 @@ public void addFolder(SpoonFolder o) {

@Override
public List<SpoonFile> getAllFiles() {
List<SpoonFile> result = new ArrayList<>();
Set<SpoonFile> result = new HashSet<>();

for (SpoonFile f : getFiles()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

f -> file

// we take care not to add a file that was already found in a folder
if (!result.contains(f)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test seems not needed anymore

result.add(f);
}
}
return result;

List<SpoonFile> resultAsList = new ArrayList<>(result);
return resultAsList;
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend to avoid unnecessary local variable:

List<SpoonFile> resultAsList = new ArrayList<>(result);
return resultAsList;

Just return what is needed:

return new ArrayList<>(result);

It is shorter and easier to understand.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, in order to improve the perf, it looks interesting to store the results locally in the instance to avoid computing it again each time. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not understand the code well enough unfortunately. Yesterday was my first day playing with spoon.

files appears to be mutable given addFile() and addFolder() methods. There is no need to call getFiles() here at all since getFiles() just returns a List representation of the Set files anyway. If there are thread concerns then there are already issues with non-protected blocks so I think just using the files as is works ok? Submitting new change today.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah actually I had second thought about my comment after posting it. So let's forgot this idea for now.

}

@Override
Expand Down