Skip to main content
Commonmark migration
Source Link

#Missing Implementation

Missing Implementation

Interfaces

#Interfaces UseUse interfaces instead of classes so clients will not be affected by the implementation.

#Naming

Naming

Directory

getContents

#Directory ##getContents YouYou 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:

numberOfFiles

##numberOfFiles YouYou are implicit doing a few things:

size

##size UsingUsing java streams will make the code shorter and more readable.

#Missing Implementation

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

#Naming

#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:

##numberOfFiles You are implicit doing a few things:

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

Missing Implementation

Interfaces

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

Naming

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:

numberOfFiles

You are implicit doing a few things:

size

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

Source Link
shanif
  • 725
  • 3
  • 9

#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.