Clever Geek Handbook

Designing code to make it easier to add tasks like this

For example, I have an application that can download videos. Since the tasks for loading are similar, I create a base class for loading.

public abstract class Download {
  public abstract void run();
}

For each specific site where videos can be uploaded, I create a child class from the base class:

public class DownloadYouTube extends Download {
  public void run() {
  }
}

public class DownloadVimeo() extends Download {
  public void run() {
  }
}

To see which site the user is loading from, I create an enum and switch it to create the correct object, then I call the generic run () method.

public enum WEBSITE {
  YOUTUBE,
  VIMEO
}

public void startDownload(WEBSITE website) {
  Download download;
  switch (website) {
    case YOUTUBE:
      download = new DownloadYoutube();
      break;
    case VIMEO:
      download = new DownloadVimeo();
      break;
  }
  download.run();
}

Later, other people will want to add new sites. It's not easy with this design. People have to edit in three places: they need to change the enum, they need to add a new case, and they need to write the class itself.

It would be better if I could just write a class.

Is there any general code design or other advice to handle this situation better than this?

+3


source to share


5 answers


Create a map! A map is a data structure that allows you to search for any value using a key, so an instance of each of the load classes can be accessed by specifying a string. (your variable "site" can turn into this)

import java.util.HashMap;
import java.util.Map;


    Map<String, Download> downloaders = new HashMap<String, Download>();

    downloaders.put("Youtube", new DownloadYoutube());
    downloaders.put("Vimeo", new DownloadVimeo());


    // Iterate over all downloaders, using the keySet method.
    for(String key: downloaders.keySet())
        Download d = downloaders.get(key)
    System.out.println();


NOTE. If you are going to use multiple instances of the same Download class, this solution will not work as shown here.

+2


source


As a possible solution, you can add an abstract factory method to your enum that would create the required object Download

. This way, WEBSITE

it becomes not just a list of the sites you maintain, but also encapsulates the behavior for each one:

public enum WEBSITE {
    YOUTUBE {
        @Override
        public Download createDownload() {
            return new DownloadYouTube();
        }
    },
    VIMEO {
        @Override
        public Download createDownload() {
            return new DownloadVimeo();
        }
    };

    public abstract Download createDownload(); 
}

public void startDownload(WEBSITE website) {
    website.createDownload().run();
}


With this approach, it will be impossible to add a new WEBSITE

one without specifying how it should be handled.

+3


source


It would be better if I could just write a class.

You can use class literals as opposed to enums:

startDownload(DownloadYouTube.class);

Just startDownload

accept Class<T extends Download>

and instantiate:

public void startDownload(Class<T extends Download> type) {
    try {
        Download download = type.newInstance();
        download.run();
    } catch(Exception e) {
        e.printStacktrace();
    }
}

Now you need to create a new class that extends / implements Download 

class DownloadYouTube extends/implements Download {

}

Download

should be interface

if it only consists of methods public abstract

:

interface Download {
    void run();
}

class DownloadYouTube implements Download {
    //...
}

The above method startDownload

would remain unchanged.

WEBSITE

should be WEBSITE

. Type identifiers must start with uppercase letters and use camel body standards:

enum Website { }

Although not required for my solution enum

.

You should check if you really need a new instance for every call. It seems like you could just pass in the url Download#run

if you've adjusted it.

If a new instance is not needed for every call:

WEBSITE

exists for easy access to various bootloaders.

This means that it YOUTUBE

must provide access to DownloadYoutube

.

You can include Download

in interface

:

interface Download {
    void run();
}

DownloadYoutube

and DownloadVimeo

could implement this:

class DownloadYoutube implements Download {
    public void run() {
        //...
    }
}

class DownloadVimeo implements Download {
    public void run() {
        //...
    }
}

WEBSITE

can also implement this:

enum Website implements Download {
    public final void run() {

    }
}

Now make it necessary for each value WEBSITE

to be specified Download

for use:

enum Website implements Download {
    YOUTUBE(new DownloadYoutube()),
    VIMEO(new DownloadVimeo());

    private final Download download;

    Website(Download download) {
        this.download = download;
    }

    @Override
    public final void run() {
        download.run();
    }
}
0


source


What you are trying to solve has been done before. There is a design pattern called the templating method

The basic idea behind the template method is that the skeleton of the algorithm is applied, but the details fall under subclasses. You have

public interface DownloadTask {
    public List<Parameter> getParameters();
    public setParameters(Map<Parameter, Object> values);
    public Future<File> performDownload();
}

with two concrete implementations

public class VimeoDownload implements DownloadTask {
  ...
}

public class YoutubeDownload implements DownloadTask {
  ...
}

or if you really want to list then

public enum DefaultDownload implements DownloadTask {
   YOUTUBE,
   VIMEO;
}

but I don't think listing will buy you as much as you might think. When trying to update an enum you must either share methods like this

 public enum DefaultDownload implements DownloadTask {
   YOUTUBE,
   VIMEO;

   public void someMethod(...) {
   }
}

or declare them individually

public enum DefaultDownload implements DownloadTask {YOUTUBE () {public void someMethod (...) {}}, VIMEO () {public void someMethod (...) {}}; }

And both scripts put you at risk of hacking all downloads in order to add a new download.

Then you have an actual template method that ties it all together

public class Downloader {

    /* asynchronous API */
    public File doDownload(DownloadTask task) {
        Map<Parameter, Object> paramValues = new Hashmap<>();
        List<Parameter> params = task.getParameters();
        for (Parameter param : task.getParameters()) {
            Object value = getValue(param);
            paramValues.put(param, value);
        }
        task.setParameters(paramValues);
        return task.performDownload();
    }

    /* synchronous API */
    public File doDownloadAndWait(DownloadTask task) {
        Future<File> future = doDownload(task);
        return future.get();
    }

}

The actual steps I provided in DownloadTask are not the ones you most likely need. Change them according to your needs.

Some people don't use the interface for DownloadTask, that's fine. You can use an abstract class like

 public abstract class DownloadTask {

     public final Future<File> doDownload() {
        this.getParameters();
        this.setParameters(...);
        return this.performDownload();
     }

     public final File doDownloadAndWait() {
        Future<File> future = this.performDownload();
        return future.get();
     }

     protected abstract List<Parameter> getParameters();

     protected abstract void setParameters(Map<Parameter, Object> values);

     protected abstract Future<File> performDownload();

 }

This is actually the best OO design, but care needs to be taken. Keep the inheritance tree shallow (one or two parents before hitting the standard Java library class) for future maintenance of the code. And don't be tempted to make custom template methods (doDownload and doDownloadAndWait). In the end, what's the key to this pattern, the operations pattern is fixed. Without this, it will hardly be possible to follow the example, and the material can become a disorderly mess.

0


source


You are heading in the right direction ...

Implementing an interface instead of extending a class will make your life easier. Also it's annoying that you need to update the enum AND change the case argument AND write a class to add a new handler.

editing 3 places is annoying, 2 is good, 1 is awesome.

We could get yours up to 2 quite easily: public enum WEBSITE { YOUTUBE(new DownloadYouTube()), VIMEO(new DownloadVimeo()) public final Download download; public WEBSITE(Download dl) { download = dl; } } public void startDownload(WEBSITE website) { website.download.run() }

Now you only edit two places: the ENUM definition and the new class. The big problem with enums is that you almost always have to edit 2 places (you need to update the enum to redirect to what you want to edit). In this case, enumeration doesn't help you at all.

My first rule of enumeration is that unless you are individually addressing each enum value elsewhere in your code, you should not use an enumeration.

You can get it up to 1 edit place, but with the new class it is quite difficult - the difficulty is that Java does not have "Discovery", so you cannot just ask about it for all classes that implement "Download".

One way could be using Spring and annotations. Spring can scan classes. The other is annotation. The worst is probably the directory containing your Load classes and trying to instantiate each one.

As it is, your solution is not bad. 2 locations are often a pretty good balance.

0


source






More articles:


All Articles