Question

Here's an example of a utility method:

public static Long getFileSize(String fileString) {

    File file = new File(fileString);

    if (file == null || !file.isFile())
        return null;

    return file.length();
}

Is it a good practise to pass a String rather than a File to a method like this? In general what reasoning should be applied when making utility methods of this style?

Was it helpful?

Solution

This is my preferred solution:

public static Long getFileSize(String path) {
    return getFileSize(new File(path));
}

public static Long getFileSize(File file) {
    return (!file.isFile()) ? -1L : file.length();
}

Note that it is returning -1L rather than 0L, to allow the caller to distinguish between an empty file, and a "file" whose length cannot be determined for some reason. The file.length() will return zero in some cases where you don't have a zero length file; e.g.

  • when the file does not exist
  • when the file is a directory
  • when the file is a special file (e.g. device file, pipe, etc) and the OS cannot determine its length.

The file.isFile() call deals with these cases. However, it is debatable whether the method(s) should return -1L or throw an exception. The answer to this debate turns on whether the -1L cases are "normal" or "exceptional", and that can only be determined with reference to the contexts in which the method is designed to be used,

OTHER TIPS

I'd go with a File. It feels a little OOP correct to me: more typesafe (Strings are so 'general' in Java...) and semantically expressive: if you are dealing with files, well then pass a File object.

Recall that in Java a File object represents not really a file in itself (its content) but rather its path: "An abstract representation of file and directory pathnames" (it can even be the path of a non-existent file) and that's almost exactly what you need here.

This can only be a limitation in a few cases: if the "file" is actually some kind of pseudo-file or resource, for example inside a jar file. An alternative I have found useful is a URI, which (conceptually) includes a File as a special case, but also other resources.

And if you decide to stick with the two alternatives (String or File), I emphatically don't recommend to name the methods the same. Method overloading can be a pain, use it only when it gives you a tangible benefit.

I would think this would depend upon what you have available at the point where you are going to be calling this method. If you have the file name (String), but no File, there seems little point in making the caller create the File from the file name.

My approach to this when I'm not sure is to create two methods, one for String, one for File. Then have the String one create the file and call the file one.

public static long getFileSize (final String fileString) {
  return getFileSIze (new File (fileString)); 
}

public static long getFileSize (File file) {
   if (file == null || !file.isFile()) return null;
   return file.length();
}

It depends on the utility expected from the client side. In case the client side already has a file object and wants to fetch the filesize, the client side developer is forced to extract the file path and pass it to the utility method. In order to avoid it, I would provide overloaded methods 1) with File 2) File Path String

Also, In case the file is unavailable, I would throw an exception than return null.

My recommendation would be to have both:

public static Long getFileSize(String path) {
    return getFileSize(new File(path));
}

public static Long getFileSize(File file) {
    return (file == null || !file.isFile()) ? 0L : file.length();
}

and let your users choose based on the the kind of object they are using to represent filesystem paths. As @Nikita mentioned, neither choice is wrong.

In my opinion, that function is only useful with a string parameter. What does it do?

  • It creates a file object.
  • Checks that it can be created.
  • Checks that it's a file
  • Returns the length

If you passed it a file, the first thing isn't needed, the next two should probably be assumed, and the length is a file member function. If you pass this a file, this function becomes too trivial to write :)

(Also, I think returning null from a function that returns a long is strange)

If you have a File object already, use:

length = file.isFile() ? file.length() : -1;

If your code deals with files instead of file names you could save yourself some file opens if you reuse the File pointers. In that case, it might lead you to use them over the filename approach.

There are a number of considerations:

  1. Utility methods exist to reduce the amount of repetitive boiler plate code in your app, hence making the code more readable and reducing the number of potential bugs. It makes sense to cater for most common usage patterns, i.e. if most of the time you have a string describing a file - pass the string. Most of the benefit comes from having a utility method in a first place, not getting the optimal signature that is 100% future-proof.

  2. Passing a file instead of a string provides stronger typing, that is to say more of you code can be checked at compile time safeguarding against typos. Make the compiler do the work for you, use the benefits of strong typing.

  3. Passing a file means that you could pass any kind of File object, possibly a bespoke in-memory file object without having to change the utility method to handle the bespoke file descriptor.

  4. Passing a string could help when you have to deal a lot with OS file paths and you just want to check the size with a minimum number of lines.

  5. At the end you could have several overloaded utility methods at a very low cost. This scenario is exactly the reason for existence of method overloading as a language feature. See what naturally works in you code base. Code is malleable and this is not one of these design decisions you'd have to live with forever, unless you're building an API for other people to use.

  6. You could also want to change the name to be a bit more descriptive, for instance

    • long sizeFromFile(File f) and
    • long sizeFromFileName (String name)

    using convention originally suggested by Joel Spolsky.

Method Overloading is the best practice in such cases.

The only thing that matters is how you're gonna use this method. In other words, if your application operates with File objects, you could pass them and remove some unnecessary operations. If you operate with file paths, string parameter may be more convenient.

But ultimately, neither choice is wrong: neither will make your application work worse.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top