• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Ron McLeod
  • Paul Clapham
  • Jeanne Boyarsky
  • Liutauras Vilda
Sheriffs:
  • Rob Spoor
  • Bear Bibeault
  • Tim Cooke
Saloon Keepers:
  • Tim Moores
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Piet Souris
Bartenders:
  • Frits Walraven
  • Himai Minh

Refactoring away from main()

 
Sheriff
Posts: 16282
271
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Starting this thread to show how one might refactor away from the "Everything is in main()" anti-pattern we often see with beginners' attempts.

In this example, I'll be using the code originally posted here: https://coderanch.com/t/743005/java/guess-word-program-hangman-correct

For convenience, this is the code as it was originally posted:
 
Junilu Lacar
Sheriff
Posts: 16282
271
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Around here, we always point out that MainIsAPain (←click that, it's a link to a Ranch Wiki page).

One problem with having everything in main() is that it's hard to isolate different concerns (tasks). That makes the code more difficult to understand. Code that has many different concerns mixed together is also harder to test in an automated way. I usually refer to code that has many concerns mixed together as "noisy" or "busy" because the intent of the code is hidden by all the implementation details.

Besides being hard to understand, the code also doesn't have any test coverage. That's a problem because now we have to manually verify that the code is doing what it's supposed to do. This can be a tedious chore and it's both time-consuming and boring. This isn't surprising though because most beginners aren't taught about testing their programs automatically until much later, if at all. In my opinion, this is one of the biggest gaps in the way many people learn about programming and it's the root of many of the problems we see in the field of programming. But that's a topic for some other time in another thread.

So, the first order of business is to put this code under test so we have a quicker and more consistent way of making sure the program does what it's supposed to do. Having automated tests will make the job of cleaning up the program much easier, faster, and safer later.

 
Junilu Lacar
Sheriff
Posts: 16282
271
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
The original code had a syntax error on the import statement so I took care of that first. Then I put it in a proper named package. Then I reformatted the code using my IDE's autoformat feature. With these preliminaries out of the way, I could start refactoring.

There are at least two things that will make this code difficult to put under test:
1. All the code is in static methods
2. It takes input from System.in

So first order of business is to move all the code that's currently in main() to a proper instance method. This is going to be a two-step process because my IDE won't extract code from a static method directly into an instance method; I have to extract the code to a static method first, then convert the static method to an instance method. Normally, you would want to have tests in place to make sure you don't break anything but in this case, we can use the IDE's automated refactoring features to make the changes relatively safe to perform.

Step 1: Extract code in main() to a static method:

Step 2: Convert the static play() method to an instance method:
 
Junilu Lacar
Sheriff
Posts: 16282
271
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
As it is now, this code is still going to be difficult to test because the instance method has a private scope. We want to make this method accessible to a test class so we'll increase the scope to package-private instead. We don't make it public because that's way too wide for what we want to do.

With that, we can exercise the code via the play() method from a JUnit test.

The next thing that is going to make it hard to test this code is the fact that it chooses a random word from an array. Random input makes for too much uncertainty. We'll need a way to disable the random behavior temporarily while we get the code under test. We'll do that by extracting the expression that provides the random word to a separate method, then override that method in a more testable version of the class. That means we have to make the extracted method protected.

The changes are on line 20 and lines 65-67 in the code above.

All the changes I've made to the code so far are relatively safe even without the benefit of tests to reassure myself that I didn't break anything. Still, I make a slight change to the code and run it manually just to prove to myself that I didn't actually break anything.


I run the program and get this as the output (bolded letters are what I entered):

Enter a letter in word ****** > f
Enter a letter in word f***** > o
Enter a letter in word fo**** > o
Enter a letter in word foo*** > b
Enter a letter in word foob** > a
Enter a letter in word fooba* > r
The word is foobar. You made 0 wrong guesses.
Do you want to guess another word? (y/n): n

Now I have a better idea of how this program behaves.
 
Junilu Lacar
Sheriff
Posts: 16282
271
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
One more thing to do before I put this baby under test: make it easier to control the input. Having input come from System.in is going to get in the way of writing automated tests. I'd rather have the input come from a String that I specify for now. So, we'll do the same thing we did with the word: we'll extract the expression to a protected method that we'll override in a more testable version of the class. Again, I make a slight change to the method and run the program manually to make sure I didn't break anything.

I run the program manually again to verify I didn't break anything. Now that I'm fairly certain I didn't break anything so far, I'll revert the small changes I made and do this from a test class instead.
 
Junilu Lacar
Sheriff
Posts: 16282
271
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Now I'm ready to write a JUnit test for this thing:

Notes:
#1 - Use this annotation to control how the test report displays the test method names

#2 - Use a subclass of GuessTheWord for testing. The only thing we'll change is the behavior of the two methods we've extracted from the main code.

#3 - override the getScanner() method and make it always return a Scanner that we specify

#4 - override the getWord() method and make it always return the same String, "foobar"

I run the test and see that the output is the same as it was in the previous step. Now we're in business!

At this point, I don't have a good unit test yet—the "test" I have above only exercises the code but doesn't assert anything about the program's behavior. That's ok for now. The win here is that I have put the means of input and the means of specifying the word to guess under control of the test. I am now able changes those at will and to run the "test" repeatedly and consistently. This will relieve me of having to enter anything at all, allowing me to iterate much faster now.
 
Junilu Lacar
Sheriff
Posts: 16282
271
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
While trying to figure out what first test to write to put some of the code under coverage, I realize that there's still something about the code that's going to get in the way: the outer do-while loop. I don't want to deal with that yes/no repeat/exit logic in my unit tests. I'd rather just test the logic for one round of the game. So, I extract the bulk of the body of that loop that's responsible for just one round of the game into another package-private method to make it accessible from the test class:

With this change, I realize that I can actually simplify my test class and get rid of the TestableGame helper class:

Once again, I run the test and see that it doesn't look like I broke anything since I get the same output as I did before except for the part that's executed by the outer do-while loop.
 
Junilu Lacar
Sheriff
Posts: 16282
271
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I'm still looking for a way to write a meaningful self-verifying test at this point. I see an opportunity. If I could just make an assertion about the result of playing one round, I'd really be in business. The problem is that the code only displays the results in local variables that I can't access from the test. Looks like I need to reintroduce the TestableGame class again and create an overridable method that I can use to get to the values I'm interested in:


Notes:
#1 (line 66 above) - create a seam for testing by extracting the System.out.println() statement to the reportResult() method. We'll override this method in a more testable version so we can get to the values of guessedLetters and wrongGuess at the end of the round.


I run the test and see that it still passes. I run with coverage to see which lines are covered by the test. Now I see my first opportunity to refactor under the protection of a test.
 
Junilu Lacar
Sheriff
Posts: 16282
271
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Since I already have part of the code covered by a test, I can start simplifying those covered parts.

This section of code could be simplified:

This could simply be written as:

I run the test after making the change and see that it still passes. Cool.

Now to simplify the next section of covered code:

I make the change (lines 33-34 below) and run the test again. The test still passes.

 
But how did the elephant get like that? What did you do? I think all we can do now is read this tiny ad:
Thread Boost feature
https://coderanch.com/t/674455/Thread-Boost-feature
reply
    Bookmark Topic Watch Topic
  • New Topic