6
\$\begingroup\$

Write a function that provides change directory (cd) function for an 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("/");
                if(this.path.endsWith("/"))
                    this.path = this.path+strarr[0];
                else
                    this.path = this.path+"/"+strarr[0];

                String newPath2 ="";
                for (int i = 1; i < strarr.length; i++) {
                    String s = strarr[i];
                    newPath2=newPath2+s+"/";
                }
                return this.cd(newPath2);

            } 

        }

    }

    private void cdup1(){
        if("/".equals(this.path)) return ;

        String [] patharr=this.path.split("/");
        System.out.println(patharr.length);
        StringBuffer s = new StringBuffer("/");

        for (int i = 0; i < patharr.length-1; i++) {
            if(!"".equals(patharr[i]))
                s.append( patharr[i]).append("/");
        }

        String endpath = s.toString();

        if(!"/".endsWith(endpath)&&endpath.endsWith("/")) endpath=endpath.substring(0,endpath.length()-1);

        this.path = endpath;
        System.out.println(this.path);

        return;
    }

    public static void main(String[] args) {
        Path path = new Path("/a/b/c/d");
        System.out.println(path.cd("../../../x").getPath());
    }
}
\$\endgroup\$

migrated from stackoverflow.com Jun 16 '16 at 19:47

This question came from our site for professional and enthusiast programmers.

6
\$\begingroup\$

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:

    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<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);
}

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.

\$\endgroup\$
4
\$\begingroup\$

I believe it would be much easier to handle if your Path object would not keep the path as String but rather as an array of the path parts (as the directories).

Path path = new Path("/a/b/c/d");

this should keep a private structure as (the split is to be done up to you)

List<String> dirs = ...
dirs.add("a");
dirs.add("b");
dirs.add("c");
dirs.add("d");

then the cd could be easier if you do the same split.

For example with the cd("../x").

// take the path /a/b/c/d from above
// the cd is splitted into cdParts ".." and "x"
for (String part : cdParts) {
    if (part.equals(".")) {
        // stay at the same level
        continue;
    } else if (part.epuals("..")) {
        // remove the last element from path
        path.remove(path.size() -1);
    } else {
        // add the cd part to the path
        path.add(part);
    }
}

It's not complete working code but it should give you a hint of the idea.

\$\endgroup\$