File System - OOP

Question

I have added an object oriented file system. I am looking for suggestions for how I could improve it further.

package oopdesign.fileSystem;

public abstract  class Entry {

    protected Directory parent;
    protected long created;
    protected long lastUpdated;
    protected long lastAccessed;
    protected String name;

    public Entry(String n, Directory p){
        name = n;
        parent = p;
        created = System.currentTimeMillis();
        lastUpdated = System.currentTimeMillis();
        lastAccessed = System.currentTimeMillis();
    }

    public boolean delete(Entry entry){
        if(parent == null) return false;
        return parent.deleteEntry(this);
    }

    public abstract int size();

    public String getFullPath(){
        if( parent == null ) return name;
        else return parent.getFullPath() + "/" + name;
    }

    /* Getter and setter */
    public long getCreationTime() { return created; }
    public long getLastUpdatedTime() { return lastUpdated; }
    public long getLastAccessed() { return lastAccessed; }
    public void changeName(String n) { name = n; }
    public String getName() { return name; }
}







package oopdesign.fileSystem;

public class File extends Entry {

    private String content;
    private int size;

    public File(String entryName, Directory directory, int size) {
        super(entryName, directory);
        this.content = content;
        this.size = size;
    }

    public String getContent() {
        return content;
    }

    public void setContent(String content) {
        this.content = content;
    }

    public int size() {
        return size;
    }

}



package oopdesign.fileSystem;

import java.util.ArrayList;

public class Directory extends Entry {

    ArrayList<Entry> fileList;

    public Directory(String entryName, Directory directory) {
        super(entryName, directory);
        fileList = new ArrayList<Entry>();
    }

    public int size() {
        int size = 0;
        for (Entry e: fileList) {
            size += e.size();
        }
        return size;
    }

    public int numberOfFiles() {
        int count = 0;
        for(Entry e : fileList){
            if(e instanceof Directory){
                count ++;
                Directory d = (Directory) e;
                count += d.numberOfFiles();
            }else if(e instanceof File){
                count ++;
            }
        }

        return count;
    }

    public void addEntry(Entry entry){
        fileList.add(entry);
    }

    public boolean deleteEntry(Entry entry){
       return fileList.remove(entry);
    }

    protected ArrayList<Entry> getContents() { return fileList; }
}

Answer 1

Seems there is an compilation error in constructor of File:

this.content = content

There is no param content in method signature.

Name params consistently (especially along inheritance hierarchy). You have p (parent) VS directory and n (name) VS entryName. Either they should be consistent throughout the hierarchy (e.g. parent and name seem to fit along) or you should deviate on purpose to highlight special cases (e.g. a future class Alias may have two constructor params with a name: aliasName and targetName).

There could be convenience methods that check for tree-attributes like:

• boolean isLeaf() always true on File, also on empty Directory
• boolean isRoot() always true on root Directory

What about consistent getter for contents:

• the naming differs between File an Directory
• the return type also differs (can this be unified with some generic contents-type?)

Naming convention for packages: avoid camelCase. What about shortening oopdesign.fileSystem to ood.fs (OOD is the abbreviation for object-oriented design) or oo.file.

In reference to the Java API there may be a name collision, because the class File is also used by Java inside package java.io.

Answer 2

Robustness

Your design lacks robustness. I.e. it relies on a specific parent-child relation described with object references but it does not enforce that the relation is sound in the data structure. You allow insertion of parentless entries and entries with different parent to a directory.

Representation of reality

Since this is a representation of a concrete idea, it should reflect the restrictions of the idea in the design. A file does not exist in a void, it always has a parent directory. Note that naming is relevant here, a document may exist as an e-mail attachment or a blob in a database but it becomes a file only when it is placed into a directory tree in a file system. So, you should prevent creation of files that are not attached to a directory by making the Entry class an interface and allowing insertion only via methods in the Directory. E.g.

public class Directory {
    public Entry createFile(String name) {
        return new ConcreteFile(...);
    }
}

With this you are in control of enforcing the integrity of the data structure. Likewise the Directory only exists in a file system. If you remove it, it becomes an archive of documents. It should be an interface and initialized via a FileSystem object.

public clas FileSystem {
    public Directory getRoot() {
        ....
    }
}

Methods for manipulating entries (copying, moving and deletion) should then be implemented in the entry, not the directory. When done this way the user is spared the complication of having to obtain a reference to the parent in order to delete an entry.

But how do you create the FileSystem? Well, the FileSystem should be an interface too, created with a FileSystemBuilder. But that's probably a subject of a separate question. :)

Misc

Integer is the wrong type to represent file sizes. It's too small. Use long.

There is no need for us to use long for temporal types anymore. It's a historical artifact from the limitations of long gone hardware. You could very well use java.time.ZonedDateTime and save yourself from a lot of hassle in the future.

For future reference: use a temporal types that include time zones whenever it is possible.

Size is a vague term in this context. In Java's collections it means the number of elements in a collection. For a direcetory I would have to look for documentation about whether it means number of files in it or not. The java.io.File class uses length. I would rename the method to "calculateSize" to signal that it does recursive calculation and can cause a whole lot of IO.

Answer 3

Missing Implementation

• lastAccessed and lastUpdated are not set beside in the entry constructor.
• a file's size is usually affected by its content.
• addEntry and deleteEntry should affect the parent of entry that was added/removed.
• Validations and error handling. For example, having 2 different files with the same name in the same directory.

---

Interfaces

Use interfaces instead of classes so clients will not be affected by the implementation.

I believe that your "implicit interface" aka the public functions are affected by the implementation. I think that if you started your coding with defining an interface, the code would be different.

Here is an example of how the interface should look like:

interface IEntry{
   
    void delete();  //no need to return anything, just perform a command
   
    void changeName(String name);
    String getName();
    
    long getSizeInBytes(); // specify the units of size

    String getFullPath();
    
    //return class instead of long
    SomeDateTimeClass getCreationTime();
    SomeDateTimeClass getLastUpdatedTime();
    SomeDateTimeClass getLastAccessed();
}
interface  IDirectory extends IEntry{
    int numberOfFiles();
    void addEntry(IEntry entry);
   
    void deleteEntry(IEntry entry);  //no need to return anything, just perform a command
           
    Iterable<IEntry> getContents(); //return the most basic type
}

Don't use status codes

Returning booleans to indicate if an operation was performed, is a simplified form of status codes. It is better to throw exceptions if the function is misused. for example, delete an entry that doesn't exist.

---

Avoiding Nulls

The problem with reference types is that they always can be Null. But how you know if they should be Null.

One solution is to use the Optional class.

Another solution is to design the classes in such a way so you don't need the Null as I described at Root Directory.

I wrote an article about this if you are interested.

---

Naming

• Don't use one letter as a name, it cannot mean anything. Also, don't use abbreviations it means too many things.

• I see you wrote this.content = content; so I wonder why not doing the same thing in Entry.

• In Entry you have function changeName and in File you have function setContent. Choose one term and stick to it.

• Directory.fileList contains also directories so it is a confusing name.

---

The Root Directory

The root directory is similar to a regular directory but also little different:

• It has no parent
• It cannot be deleted

So I think it should have its own class, let's call it RootDirectory.

No parent

In your current design, when the parent field is null it means this is the root directory. Based on that you have different logics for getFullPath and delete.

If the root directory doesn't have a parent why it should include this field? writing the specific code for the root directory in RootDirectory class simplifies the code.

delete

If the root directory cannot be deleted why it should have a delete function?

I suggest splitting the interface to IDeleteable and IEntry. File and Directory should implement IDeleteable, RootDirectory should not.

---

Directory

getContents

You return a reference to fileList which is a member of Directory class. The return type is ArrayList. Let's imagine someone using your classes and wrote the following code:

ArrayList childs = directory.getContents()
childs.Add(someEntry)

the result of this code is that fileList is changed without the Directory class knowing about it. Returning a "read-only type" avoids this problem. Iterable is "read-only type" since it only allows iterating.

I didn't understand why getContents is protected and not public.

numberOfFiles

You are implicit doing a few things:

• Split between files and directories
• Count the files
• Sum the count of all directories
• Sum the 2 above

I think writing the code in the above way is more readable and using Java Streams it should be very easy.

size

Using java streams will make the code shorter and more readable.