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.

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

Code quality - drop while(true) #526

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Ayowel
Copy link
Contributor

@Ayowel Ayowel commented Jul 18, 2021

Fix #524 and added comments on the loops where a while(true) seemed legitimate to justify them.

Note that:

  • The file watcher now resets right after polling to avoid useless consecutive calls
  • In FileBrowseServiceImpl.searchDownAndSelect, I replaced the use of previous by next in if (Objects.isNull(searchFoundItem)) as I could not make sense of the code the way it was.

Other than that, old and new code should be strictly equivalent

}
} else {
break;
while (Objects.nonNull(currentParent)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this should be replaced by

for (Path currentParent = optional.get(); Objects.nonNull(currentParent); currentParent = currentParent.getParent())

@Ayowel Ayowel changed the title Fix/no while true Code quality - drop while(true) Jul 19, 2021
@Ayowel
Copy link
Contributor Author

Ayowel commented Jul 19, 2021

Updated the while loop to a for in DirectoryServiceImpl as it is more accurate to what the code does.

Also removed the class' constructor as well as some attributes that were not used. Dropped a lambda instantiation as the raw value it returned could be used instead and avoided the use of a stream to create a list where the list could directly be instantiated instead.

} else if (kind == ENTRY_MODIFY && event.count() > 1) {
watchKey.reset();
} else {
} else if (kind != ENTRY_MODIFY || (kind == ENTRY_MODIFY && event.count() == 0)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FIXME: should be && event.count() <= 1

@@ -118,6 +120,7 @@ private void watchPathChanges() {
}

List<WatchEvent<?>> watchEvents = watchKey.pollEvents();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pollEvents is not blocking, might want to add a sleep somewhere in here or use a self-scheduling method instead of a loop

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.

Needless processing and confusing logic in source code
1 participant