HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Resolving paths

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pathsresolvingstackoverflow

Problem

We're using a class called Places (inspired by Allen Holub's great article on DrDobbs) that resolved our program paths based on the existence of a configuration file, environment variable or neither. In the neither case, we want all paths to default to resources within our package, the .jar. Additionally, we want to force local paths if we are in a testing environment because of the complexity in handling the Lucene Database.

The original code looks like this:

```
private void resolvePaths() {
for (NewPlaces place : values()) {
List probablePaths = new ArrayList<>();

try {
if (! IS_IN_A_UNIT_TESTING_ENVIRONMENT && !place.forceLocalPaths) {
File configuration = new File("places.properties");

if (configuration.exists()) {
LOGGER.info("The configuration file exists!");
Properties properties = new Properties();
properties.load(new FileInputStream(configuration));

probablePaths.add(place.path.replace("~", properties.getProperty("mediaplatform.home")));
}

if (System.getenv("MEDIAPLATFORM_HOME") != null) {
String environmentValue = System.getenv("MEDIAPLATFORM_HOME");

probablePaths.add(place.path.replace("~", environmentValue));
}
}
} catch (IOException e) {
throw new RuntimeException("The file could not be read.");
}

String localPath = place.path.replace("~/", "");
URL localUrl = Thread.currentThread().getContextClassLoader().getResource(localPath);
if (localUrl != null)
probablePaths.add(localUrl.getPath());

for (String probablePath : probablePaths) {
File file = new File(probablePath);
if (file.isDirectory()) {
LOGGER.info("\t{} -> {}", place.path, probablePath);
place.path = probablePath;

Solution

Due to the size of the code I think you'd be better off just adding some comprehensive comments rather than refactor.

So to answer your question:

Did you needlessly complicate it? Yes and No

Yes: It is needless since the code already worked as intended.

No: You didn't complicate it.

So you spent some time for an un-necessary refactor, but if it helped you understand the code then that is a plus.

Context

StackExchange Code Review Q#29695, answer score: 3

Revisions (0)

No revisions yet.