Good object oriented design example in Java

Question

I have a Java class called TestExecutor which responsible for starting a test. Starting the test involves a number of stages:

- Update test repository
- Locate the test script
- Create result empty directory
- Execute command
- Parse output
- Update database

For each of these stages I have created private methods in the TestExecutor class which perform each of the actions above, all surrounded in a try-catch block. I'm aware that this is not good design as my class does too much and is also a pain to unit test due to a large amount of functionality being hidden in private methods.

I'd like to hear your suggestions for refactoring this class as I'm not sure how to get away from something similar to the above structure. Code example below:

public void start() throws TestExecuteException {
    try {
        updateRepository();
        locateScript();
        createResultDirectory();
        executeCommand();
        parseOutput();
        updateDatabase();
    catch(a,b,c) {
    }
}

private updateRepository() {
    // Code here
}
// And repeat for other functions

why are your methods private not public? There is no problem in having multiple methods in a single class as long as the class is having a single responsibility. — underdog, Jan 24 '17 at 16:24
There is no reason for the methods to be called by anything external to TestExecutor so I decided to make them private. My problem is that the class has a single general responsibility (test execution) however there are multiple responsibilities underneath that (update registry, locate script etc.) — Adam, Jan 24 '17 at 23:00
@Adam Any specific reason you don't want to comment on the answers or accept one of them? Voting up is free. Accepting an answer gives you two points. Do let me know if you have any concerns that your stopping you from accepting any of the answers. — Chetan Kinger, Feb 20 '17 at 15:23

score 1 · Answer 1 · answered Feb 05 '17 at 14:30

I would do this way. First, enforce the contract that each test step should have.

interface TestCommand{
  void run();
}

Now make your test commands as separate classes. Try to make these commands classes generic so that you reuse for similar types of commands. Now in your class where you want to run a test, configure the test steps as following.

//in your test class do this.
List<TestStep> testCommands = new ArrayList<>();
testCommands.add(new UpdateRepoCommand());
testCommands.add(new LocateScriptCommand());
// and so on....

Now, execute all your steps chronologically.

public void start(testSteps) throws TestExecuteException {
    try {
        for(TestCommand command : testCommands){
           command.run()
    }        
    catch(Exception e) {
      //deal with e
    }
}

Moreover, as CKing described above, follow SOLID principle inside those test steps. Inject dependencies and write the unit test for them separately.

score 0 · Answer 2 · edited May 23 '17 at 11:54

Well your class looks ok to me.

I'm aware that this is not good design as my class does too much

As far as the class has a single responsibility the number of methods don't matter.

Check this template method design pattern. Your class is doing something similar to what the abstract Game class is doing.

public abstract class Game {
   abstract void initialize();
   abstract void startPlay();
   abstract void endPlay();

   //template method
   public final void play(){

      //initialize the game
      initialize();

      //start game
      startPlay();

      //end game
      endPlay();
   }
}

and is also a pain to unit test due to a large amount of functionality being hidden in private methods

Read this & this about testing private methods. You can also use a framework like PowerMock which helps you in testing untestable code.

The class seems to have multiple responsibilities. 1) Talk to the database. 2) Talk to directories. 3) Parse data and create output. — Chetan Kinger, Feb 05 '17 at 14:30

Chetan Kinger · Answer 3 · 2017-02-05T14:14:35.590

I'd like to hear your suggestions for refactoring this class as I'm not sure how to get away from something similar to the above structure

You should definitely take a look at the SOLID principles as a starting point to writing clean,testable object oriented code. I was introduced to it at the begning of my career and it really helps a lot to follow these principles.

That said, I would start by grouping related functionality into different classes. For example, updateRepository() and updateDatabase() can be moved to a separate class called DatabaseHelper. Similarly locateScript() and createResultDirectory() seem to be disk related operations and can be moved to a seperate class called DirectoryHelper. I believe you get the gist of it. What you just achieved was Seperation of Concerns.

Now that you have separate classes, you need to bring them together and put them to work. Your TestExecutor can continue to have the methods that you have listed. The only different will be that these methods will now delegate their work to the individual classes that we created above. For this, TestExecutor will need a reference to the DatabaseHelper and DirectoryHelper classes. You could just instantiate these classes directly inside TestExecutor. But that would mean that TestExecutor is tightly coupled to an implementation. What you can do instead is ask code outside TestExecutor to supply the DatabaseHelpe and DirectoryHelper to use. This is known as Dependency Inversion through Dependency Injection. The advantage of this approach is that you can now pass any subclass of DatabaseHelper and DirectoryHelper to TaskExecutor and it doesn't have to know the details of the implementation. This facilitates in the unit testing of TaskExecutor by mocking these dependencies instead of passing actual instances.

I will leave the rest of the SOLID principles for you to explore, implement and appreciate.

Good object oriented design example in Java

3 Answers3