开发者

Reduce memory foot print by replace anonymous class with singleton. But need to refactor the design more

开发者 https://www.devze.com 2023-03-10 09:48 出处:网络
So I have this method that get executed repeatedly public static boolean isReady(String dirPath, int numPdfInPrintJob){

So I have this method that get executed repeatedly

public static boolean isReady(String dirPath, int numPdfInPrintJob){
    File dir = new File(dirPath);
    String[] fileList = dir.list(new FilenameFilter(){
        public boolean accept(File file, String filename) {
            return (filename.toLowerCase().endsWith(".pdf"));
        }
    });
    if(fileList.length >= numPdfInPrintJob) return true;
    else return false;  
}

This method using anonymous class that will create a new instance of FilenameFilter every time invoked and I invoke this method a lot. So I want to make this anonymous class into a singleton. So my initial thought is to create a new singleton class that look like this

public class PdfFileNameFilter implements FilenameFilter{
    private PdfFileNameFilter(){} //non-instantible

    //guarantee to only have one instance at all time
    public static final PdfFileNameFilter INSTANCE = new PdfFileNameFilter();

    public boolean accept(File dir, String name) {
        return (name.toLowerCase().endsWith(".pdf"));
    }
}

Can I refactor this a bit more. I need to do ZipFileNameFilter as well, and maybe many different file extension filter. Dont want to create a class for each filter. I need to refactor this开发者_JAVA百科 design a bit more. Maybe interface come into place somewhere here.


If all you wanted to do was reduce memory usage you could have done

private static final FilenameFilter PDF_FILES = new FilenameFilter(){
    public boolean accept(File file, String filename) {
        return (filename.toLowerCase().endsWith(".pdf"));
    }
}

If you want to create a singleton, the simplest way is

public enum PdfFileNameFilter implements FilenameFilter {
    INSTANCE;

    public boolean accept(File dir, String name) {
        return (name.toLowerCase().endsWith(".pdf"));
    }
}


It seems simpler to me to just use your existing anonymous class and make one instance that all your method invocations use.

private static final FilenameFilter PDF_FILTER = new FilenameFilter() {
    public boolean accept(File file, String filename) {
        return (filename.toLowerCase().endsWith(".pdf"));
    }
}

public static boolean isReady(String dirPath, int numPdfInPrintJob){
    File dir = new File(dirPath);
    String[] fileList = dir.list(pdfFilter);
    if(fileList.length >= numPdfInPrintJob) return true;
    else return false;  
}

This is a case where subclassing and making a singleton seems to be a tad overkill: you simply want only one instance to use right here, whereas a singleton is used when there is only one instance you will ever want to use.


You could use an enum for this:

public enum ExtensionFilter implements FilenameFilter {
    PDF(".pdf"),
    ZIP(".zip");

    private final String extension;

    private ExtensionFilter(String extension) {
        this.extension = extension;
    }

    @Override
    public boolean accept(File dir, String name) {
        return (name.toLowerCase().endsWith(extension));
    }
}

Now you'll be able to use it like:

dir.list(ExtensionFilter.PDF)

You can also iterate through them if you need:

for ( FilenameFilter fileNameFilter : ExtensionFilter.values() ) {
    ....
}

You could also use a vararg as constructor argument to allow multiple extensions for the same filter and use the constant name as the default to make it simpler to use:

public enum ExtensionFilter implements FilenameFilter {
    PDF,
    ZIP(".zip", ".jar", ".war", ".ear");

    private final String[] extensions;

    private ExtensionFilter(String... extensions) {
        if (extensions.length == 0) {
            extensions = new String[] {"." + name().toLowerCase()};
        }
        this.extensions = extensions;
    }

    @Override
    public boolean accept(File dir, String name) {
        for (String extension : extensions) {
            if (name.toLowerCase().endsWith(extension)) {
                return true;
            }
        }
        return false;
    }
}


you can step away from the full singleton and use a private field to set up the extension

public class ExtensionFileNameFilter implements FilenameFilter{
    private String extension;
    private ExtensionFileNameFilter (String extension){this.extension=extension;} 

    public static final ExtensionFileNameFilter PDFINSTANCE = new ExtensionFileNameFilter (".pdf");
    public static final ExtensionFileNameFilter ZIPINSTANCE = new ExtensionFileNameFilter (".zip");
    //add instances as you need

    public boolean accept(File dir, String name) {
        return (name.toLowerCase().endsWith(extension));
    }
}


Can I refactor this a bit more.

Yes, yes you can.

Assuming that wasn't the answer you were looking for (you should update yoru question to ask a more specific question), I wouldn't refactor it until you need it; YAGNI.

Once you have more code, like more FilenameFilters, is when you will see the possible refactorings. You will see common code, maybe an interface, stuff like that. Don't try to pre-maturely over-engineeer.

TDD is also the best way to do refactoring safely. If you have tests showing what code you actually need, lots of the extra stuff goes away, if any. And with a comprehensive test suite, you can refactor without hesitation because you know if your changes work or not based on whether your tests continue to pass.


For interest, this alternative accept implementation would run much faster in a benchmark test. It does not create new stateful Objects or carry the other overhead of String.toLowerCase, which is not required for your case.

  public boolean accept(File file, String filename) {
    int offset = s.length() - 4;
    if (offset >= 0) {
      if (s.charAt(offset) == '.') {
        offset += 1;
        if (s.regionMatches(offset, "pdf", 0, 3)) {
          return true;
        } else if (s.regionMatches(offset, "PDF", 0, 3)) { 
          return true;
        }
      }
    }
    return false;
  }

If this was an execution hotspot and you were looking for optimization, something like that would help.

0

精彩评论

暂无评论...
验证码 换一张
取 消