0

When should I use a File and when should I use a String with a filename as constructor? The constructed method should contain the contents of the file. So I can choose:

public GraphModel(File file) throws IOException {
    this();
    readFromFile(file);
}

versus

public GraphModel(String filename) throws IOException {
    this();
    readFromFile(new File(filename));
}

Is one preferred over the other?

Bart Louwers
  • 873
  • 9
  • 28
  • 2
    It's not uncommon to have both – Kevin Le - Khnle Jun 10 '16 at 23:53
  • Depends on the code that calls this constructor – OneCricketeer Jun 10 '16 at 23:55
  • or `public GraphModel(Readable readable) {`. But key in my mind is responsibility. What is the responsibility of the GraphModel class? Is it to read in from the file? Is it to create a graphics model? If the latter, then given the "single responsibility rule", I'd recommend having file IO separate from this class. – Hovercraft Full Of Eels Jun 10 '16 at 23:55
  • Don't make constructor, make factory methods in your case... – Olivier Grégoire Jun 11 '16 at 00:02
  • have a look here,
    [difference between passing an object and a primitive datatype as parameter](http://stackoverflow.com/questions/29997881/what-is-the-difference-between-passing-an-object-and-a-primitive-data-as-the-par)
    – isamirkhaan1 Jun 11 '16 at 00:05
  • Thanks for the helpful answers. I hope the person that downvoted me can give an explanation. – Bart Louwers Jun 11 '16 at 00:16
  • @ultrabowser - Asking "what is best practice for XXX" is asking for downvotes. http://meta.stackexchange.com/questions/145499/how-to-ask-best-practice-questions. The phrase "best practice" in programming is often taken as a "red flag" that the OP wants a recipe that they can apply without thinking about the real problem and the context. – Stephen C Jun 11 '16 at 01:26
  • For example, if you read my Answer carefully, you will see that it is actually saying "it depends ....". In other words, there is NO simple "best practice" here, even if you accept "best practice" as a sound concept. – Stephen C Jun 11 '16 at 01:31
  • @StephenC Thanks. Removed "best practices" from my question to be more in line with the link you shared. – Bart Louwers Jun 11 '16 at 01:37

3 Answers3

2

Some people think this is a matter of taste / personal preference ... or that it doesn't matter. I would argue that it DOES matter.

Lets assume that your API's implementation does require a File internally. Then, there are three alternative approaches:

  // #1
  public GraphModel(File file) throws IOException {...}

  // #2
  public GraphModel(String file) throws IOException {...}

  // #3
  public GraphModel(File file) throws IOException {...}
  public GraphModel(String file) throws IOException {
      this(new File(file));
  }

Now compare these APIs in various contexts:

  • If you adopt approach #1, and the caller has the filename as a String it must construct a File. This potentially results in duplicative code1.

  • If you adopt approach #2, and the caller has the filename as a File it must use File.toString() to get the argument for the constructor. This is potentially duplicative AND it is inefficient2.

  • If you adopt approach #3, the API works well in either case.

So my conclusion is:

  • If you are trying to provide a general API for use in contexts where you don't know what form the caller will have the filename, then approach #3 is clearly the best, and approach #2 is better than approach #1.

  • If your API is not general; i.e. you are designing for a specific context, then either approach #1 or #2 could be OK ... if they fit your intended usage patterns. However, you might3 also want to design your application so that you don't keep on creating File objects for the same filenames.


1 - If your constructor is called in lots of places, that could mean a new File(...) call at each place.

2 - The caller must convert its File to a String, only to have your constructor create a new File from the String.

3 - But ONLY if this aspect of the application is demonstrably performance critical.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • Thank you for your extensive answer. Since this constructors is not called in a lot of places, and I could use the single String argument constructor for another purpose, I'm probably best off with option `#1`. – Bart Louwers Jun 11 '16 at 01:32
1

Imho it's not important. I've often overloaded the constructor and let the constructor with the String parameter call the one with the File parameter to allow for both. Which won't work of course if you want to call the parameterless constructor as well. Otherwise it's like

public final class GraphFile {

    private final File file;

    public GraphFile(String fileName) {
        this(new File(fileName));
    }

    public GraphFile(File file) {
        this.file=file;
    }

    public GraphModel read() throws IOException {
        return whatEverReadsYourGraphModel(file);
    }
}
Arjan
  • 823
  • 1
  • 7
  • 18
0

I think that your counstructor should be implementation-of-data-reading independent. What if some day you'll be forced to read your data directly from user? Then you'd have to make some changes in your constructor.

I'd suggest making your constructor accepting argument which is for example list of verticies(I suppose they are verticies). Then, outside the constructor you can process your data independently and initialize your object with it.

threaz
  • 403
  • 7
  • 14