5

I have recently been thinking about the following design consideration: let's say I have an object that is able to read in a file and return some result. Would you say this object should rather expose an interface:

void readFromFile(File file);

or would you design it to have a method

void readFromFile();

and provide necessary values in constructor? The second option seems to be fine if we want to have multiple parameters for constructor and use a builder to build fileReaders based on some user preferences... What do you think?

Improve this question
2
  • The second option smells like a static method. And, while they have its uses, abusing them is a good way for non OOP in Java. Commented Nov 19, 2012 at 0:01
  • 5
    @SJuan76 I assume you mean the first option smells like a static method? Commented Feb 20, 2015 at 21:33

4 Answers 4

Reset to default
9

It depends on the wider context of the object.

If your object is highly cohesive, i.e. it has a narrow scope in its purpose which is primarily to read from a particular file, then it should be defined in the constructor. In terms of OO design, high cohesion is generally a good thing - so I'd generally favour this option. If your application is multi-threaded and thus your object needs to be thread safe, then I'd definitely lean towards this - I'd argue this approach makes it easier to build granular, immutable objects which is a great help when trying to avoid race hazards.

If your object is responsible for many other tasks, you don't really need to worry about concurrency, and it doesn't really make sense for the file to be included as part of the state of the actual object, then the method that takes the parameter would arguably be the best choice.

Improve this answer
Sign up to request clarification or add additional context in comments.

8 Comments

as per my comment above - in a case of storing further read statistics and using in a multithreaded env, would option 2 be better?
See edit - in a multithreaded environment, I'd definitely go for option 1.
Well, if an object also stores Load statistics, then a single method would not make sense right? Multiple callers weould potentially see one shared results, not necessarily theirs, depending on race condition...
Hmm, I could wrap the result and load stats into one object I guess, and return it
@Bober02 I mean I would separate the objects out, keep the reader just as a reader (highly cohesive) and then pass it into your other object that deals with the rest of the logic.
|
2

I think this is a question about what your object is representing.

You should wonder yourself if it has any meaning to have an instance of what your object models without the file.

Think about your object's responsibility, use the "taking the object to the desert" method: try to think what your object is, and what things it should know. Just then you'll have your answer.

Improve this answer

Comments

1

First solution is the cleanest.

However, there's a big difference when dealing with multithreading.

Two cases:

Your class is supposed to be instantiated once:

If your object isn't immutable and own file object as a field, a lot of synchronisations (locks) has to be made to avoid surprises.

Your class is supposed to be instantiated N times

N times for N threads => may be demand a lot of memory depending on your requirement and the weight of the instance.

Thus, the API void readFromFile(File file); is more suitable since file would be local to each thread.

Actually, if your class owns a lot of methods manipulating the file object, and concurrency and memory spaces aren't the primary priority, make the file an instance's field. This would clean your code.

Improve this answer

3 Comments

what a bout a stored Load statistics? In a multithreaded env, this would make a mess, as each caller would see (potentially) the same result, not necessarily for the load they submitted, but the last call tot this method
@Bober02 I don't see what you call "Load statistics". And in which case, the first or the second way?
@Bober02 I now understand what you want (by reading comments above). So, two solutions: either creating one instance per read file, and make one additional field pointing to a class named like: Statistics containing retrieved information of your file. Either creating a ConcurrentHashMap in a third object, supposed to collect all Files (key) with its Statistics (values). This map would be access by your class's methods and the whole allowing you to instantiate just one instance! So first solution works also.
0

It really depends where this object is going to be used- if this is for a program where there are only one or two files being read, and they're being read a number of times, then the second option would be better; in almost all other scenarios, the first would prevail, as you could use a single object to read a number of files.

Then again, if you plan on using this class in a number of programs (or even distributing it) and therefore are unsure of which method would be best beforehand, there's no reason why you can't include both- have two constructors (one default one, and one that takes a file), and then include both of the methods you listed above- that way the class could be effectively used in either type of program.

i.e.

public class YourFileReader() {
    File defaultFile;

    public YourFileReader() {
        //initialize
    }

    public YourFileReader(File aDefault) {
        defaultFile = aDefault;
        //initialize
    }

    public void readFromFile() {
        if(defaultFile!=null) {
            //read from defaultFile
        }
        else {
            //exception?
        }
    }

    public void readFromFile(File file) {
        //read from file
    }
}
Improve this answer

5 Comments

Well, I was planning to reuse it with the following criteria in mind: each object would read in one fle, and then store statistics of that read in a separate object, as part of its internal state. Also, I was planning to use this object in a multithreaded env - read in many files concurently. Would option 2 make sense in this scenario?
-1: Two ways of doing the same thing? Just design your model correctly (Note: this has nothing to do with overloaded constructors).
Both options should work perfectly fine in a threaded application, but as for storing statistics about that file, the second option would be much better. However, depending on the complexity of your FileReader class, you may end up using a lot of extra memory, depending on how many files you're reading- if your only reason for using the second option would be to store file information, you might be better off storing the information somewhere else (an array or list kept elsewhere) instead of keeping a bunch of FileReaders around. Depends entirely on your program, just a consideration.
@SJuan76, Mr. Bober02 never specified whether or not this was just for a single program he was making- I was giving him the idea in case he was planning on using this class for a number of programs, or possibly even distributing it, in which case being able to choose which method to use without remaking the class would be useful. I see the confusion though, I'll edit my post, thanks :)
Are you saying that you want the user to remember to use methodA or methodB depending if he called constructorA or constructorB? Either the method depends of the object state of it does not (but then, why use an instance method), but "both" is not a good option.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.