patternjavaMinor
Simulate the "cd" command of a file system
Viewed 0 times
thefilesystemcommandsimulate
Problem
Write a function that provides change directory (cd) function for an
abstract file system.
Notes:
For example,
Note: The evaluation environment uses '\' as the path separator.
It works, but is there a better way to do this?
```
public class Path {
private String path;
public Path(String path) {
this.path = path;
}
public String getPath() {
return path;
}
public Path cd(String newPath) {
//1step consume newPath
//newPath to list
//take 1 char, head pull
//locate it with this.path,
// case / ../ ./
// this.path end poll
//return changed path and newPath
//recursive cd again
if( "".equals(this.path)||"".equals(newPath)||!this.path.startsWith("/"))
return this;
else if(newPath.startsWith("/")) {
this.path = "/";
return this.cd(newPath.length()>1?newPath.substring(1):"");
}
else{
if(newPath.startsWith("..."))
return this;
else if(newPath.startsWith("../")) {
this.cdup1();
return this.cd(newPath.length()>3?newPath.substring(3):"");
}else if(newPath.startsWith("./")) {
return this.cd(newPath.length()>2?newPath.substring(2):"");
}
else if(newPath.startsWith("..")) {
this.cdup1();
return this.cd(newPath.length()>2?newPath.substring(2):"");
}else if(newPath.startsWith(".")) {
return this.cd(newPath.length()>1?newPath.substring(1):"");
}
else {
String [] strarr = newPath.split("/");
abstract file system.
Notes:
- Root path is '/'.
- Path separator is '/'.
- Parent directory is addressable as "..".
- Directory names consist only of English alphabet letters (A-Z and a-z).
For example,
new Path("/a/b/c/d").cd("../x").getPath() should return/a/b/c/x.Note: The evaluation environment uses '\' as the path separator.
It works, but is there a better way to do this?
```
public class Path {
private String path;
public Path(String path) {
this.path = path;
}
public String getPath() {
return path;
}
public Path cd(String newPath) {
//1step consume newPath
//newPath to list
//take 1 char, head pull
//locate it with this.path,
// case / ../ ./
// this.path end poll
//return changed path and newPath
//recursive cd again
if( "".equals(this.path)||"".equals(newPath)||!this.path.startsWith("/"))
return this;
else if(newPath.startsWith("/")) {
this.path = "/";
return this.cd(newPath.length()>1?newPath.substring(1):"");
}
else{
if(newPath.startsWith("..."))
return this;
else if(newPath.startsWith("../")) {
this.cdup1();
return this.cd(newPath.length()>3?newPath.substring(3):"");
}else if(newPath.startsWith("./")) {
return this.cd(newPath.length()>2?newPath.substring(2):"");
}
else if(newPath.startsWith("..")) {
this.cdup1();
return this.cd(newPath.length()>2?newPath.substring(2):"");
}else if(newPath.startsWith(".")) {
return this.cd(newPath.length()>1?newPath.substring(1):"");
}
else {
String [] strarr = newPath.split("/");
Solution
Overcomplicated
The implementation is really overcomplicated and hard to read.
It works, but I feel there must be better way to do that?
Your instincts are good. When you know there's a better way, something you can try is start over from scratch using a different algorithm and see where that goes. Another technique is using TDD and let the tests drive your design.
Poor readability
The code is too compactly written at many places, for example:
This is really hard to read like that and doesn't help the understandability of the code.
All modern IDE's have an auto-reformat function that will put spaces around operators and make the code much easier to read.
Normalization
The
This is a bit confusing inconsistency. It would be better to normalize the initial path too.
With a good normalizer method, the implementation of
In this case, of course,
It could go something like this:
Bug or unclear specs?
What should
I would expect
What about
I would expect
Special treatment of
There is a condition for path segments starting with
That makes this special treatment odd.
It seems that such path segment is illegal, and throwing an
Testing
For something a bit tricky like this,
the
Not to mention of course that testing in a
I strongly recommend to develop test cases and cover as many interesting corner cases as possible, to flush out obscure bugs.
The implementation is really overcomplicated and hard to read.
It works, but I feel there must be better way to do that?
Your instincts are good. When you know there's a better way, something you can try is start over from scratch using a different algorithm and see where that goes. Another technique is using TDD and let the tests drive your design.
Poor readability
The code is too compactly written at many places, for example:
if( "".equals(this.path)||"".equals(newPath)||!this.path.startsWith("/"))
// ...
return this.cd(newPath.length()>1?newPath.substring(1):"");
// ...
return this.cd(newPath.length()>3?newPath.substring(3):"");This is really hard to read like that and doesn't help the understandability of the code.
All modern IDE's have an auto-reformat function that will put spaces around operators and make the code much easier to read.
Normalization
The
cd method effectively normalizes the path, for example a/b/.. becomes correctly a. But the initial path passed to the constructor is never normalized. For example new Path("/a/b/../..").cd("c").getPath() returns /a/b/../../c instead of c.This is a bit confusing inconsistency. It would be better to normalize the initial path too.
With a good normalizer method, the implementation of
cd itself could be much simpler:public Path cd(String newPath) {
if (isAbsolutePath(newPath)) {
path = normalizePath(newPath);
return this;
}
path = normalizePath(path + SEPARATOR + newPath);
return this;
}In this case, of course,
normalizePath does the real heavy lifting.It could go something like this:
private String normalizePath(String path) {
boolean isAbsolute = isAbsolutePath(path);
List parts = new ArrayList<>();
for (String part : path.split(SEPARATOR)) {
if (part.isEmpty() || part.equals(".")) {
continue;
}
if (part.equals("..")) {
if (parts.isEmpty()) {
if (isAbsolute) {
continue;
}
} else {
if (!parts.get(parts.size() - 1).equals("..")) {
parts.remove(parts.size() - 1);
continue;
}
}
}
parts.add(part);
}
String prefix = isAbsolute ? SEPARATOR : "";
return prefix + String.join(SEPARATOR, parts);
}Bug or unclear specs?
What should
new Path("a/b").cd("..").getPath() return?I would expect
a, but in fact it gives a/b.What about
new Path("a/b").cd("../../..").getPath()?I would expect
.., but it gives a/b for this too.Special treatment of
...There is a condition for path segments starting with
.... Why is that? The description says that directory names contain only letters of the alphabet.That makes this special treatment odd.
It seems that such path segment is illegal, and throwing an
IllegalArgumentException would seem appropriate.Testing
For something a bit tricky like this,
the
main method in the class is really far too little to verify that the class works well.Not to mention of course that testing in a
main method is not a good way.I strongly recommend to develop test cases and cover as many interesting corner cases as possible, to flush out obscure bugs.
Code Snippets
if( "".equals(this.path)||"".equals(newPath)||!this.path.startsWith("/"))
// ...
return this.cd(newPath.length()>1?newPath.substring(1):"");
// ...
return this.cd(newPath.length()>3?newPath.substring(3):"");public Path cd(String newPath) {
if (isAbsolutePath(newPath)) {
path = normalizePath(newPath);
return this;
}
path = normalizePath(path + SEPARATOR + newPath);
return this;
}private String normalizePath(String path) {
boolean isAbsolute = isAbsolutePath(path);
List<String> parts = new ArrayList<>();
for (String part : path.split(SEPARATOR)) {
if (part.isEmpty() || part.equals(".")) {
continue;
}
if (part.equals("..")) {
if (parts.isEmpty()) {
if (isAbsolute) {
continue;
}
} else {
if (!parts.get(parts.size() - 1).equals("..")) {
parts.remove(parts.size() - 1);
continue;
}
}
}
parts.add(part);
}
String prefix = isAbsolute ? SEPARATOR : "";
return prefix + String.join(SEPARATOR, parts);
}Context
StackExchange Code Review Q#132229, answer score: 7
Revisions (0)
No revisions yet.