Skip to content

Conversation

@chenzimin
Copy link
Contributor

All path operations are now handled by java.nio.file.Path instead of string concatenation.

@electricalwind electricalwind added the question Further information is requested label Jan 11, 2019
@electricalwind
Copy link
Owner

Hello,
Before integrating the change, i would like to know whether the suggested changes were made in order to correct a bug.

The use of Java nio resolve is a bit more costly in comparison of the previous String interface and force the use of toString methods everywhere including in some of my projects extending this tool and overall hinder the global readability.

I don't really mind the change and will likely integrate it, but I would like to know its motivation.
Best,

@chenzimin
Copy link
Contributor Author

chenzimin commented Jan 11, 2019

Hi,

I made this change because when using data7 for the first time, I specified my path this way:

ResourcesPath path = new ResourcesPath(/foo/bar);

This caused binaryPath in ResourcesPath.java to be /foo/barbinary/, due to string concatenation, and caused exceptions to be thrown. It took me a while to figure this out.

Path library can also handle file separator for Linux and Windows, / and \ etc.

There are several advantages of handling path with library instead of string. This PR is intended to avoid future bugs and improve usability.

Regards,
Zimin

@chenzimin
Copy link
Contributor Author

chenzimin commented Jan 11, 2019

I might also do another PR to increase the usage of java.nio.file.Path, so that we can avoid calling toString() everywhere. For example changing Utils.checkFolderDestination(String) to Utils.checkFolderDestination(Path).

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

Labels

question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants