Using TDD and trying to make a simple calculator.

[Junilu Lacar]

Here's a useful hint: You can brute force the logic for checking the winner between two moves with a bunch of if-statements OR you can use this little algorithm:

Assigning ranks Rock=1, Paper=2, Scissors=3, given Move1 and Move2, calculate winner = (3 + Move1.rank - Move2.rank) % 3 ==> winner will be 0 if Move1 == Move2, Move1 wins if winner is 1, Move2 wins if winner is 2. This can be written as one line of Java code or if you want it to be a little clearer, 4 or 5 lines.

[Junilu Lacar]

Ignoring that hint about how to calculate the winner, your first cut at the Move enum should be about 6 lines of code. That should be enough code to see the test code compile and fail.

[Junilu Lacar]

Here's another glimpse into my thought process. I'm envisioning writing something like this later on:

Move computer = simulator.move();
if (myMove.equals(computer)) {
   // it's a tie!
} else if (myMove.beats(computer)) {
   // player wins!
} else {
   // computer wins!
}

That's why I started with a test that would exercise the Move.beats() method. Alternatively, I could have started with a test around a tie but that's not very interesting. It would have been a smaller step though, requiring only about two or three lines of code.

Edit: That code is actually a direct translation of what I wrote earlier about my thought process. When you put yourself in the mindset of "storyteller", writing code becomes very conversational and, IMO, the resulting code is very readable.

I wrote:The result will be either a tie when the computer and I make the same move, or I will win, or the computer will win.

[Junilu Lacar]

In the refactoring step, here's the question I'd ask: "Is 'Move' a good name? Does it fit with the nomenclature of the Rock-Paper-Scissors game? Can we find a better name?"

If you read through the Wikipedia article on Rock Paper Scissors, there are at least two other alternative names we can try to use: "Shape" and "Gesture". I would try both of them out and see how the code reads. Don't just imagine it though; actually change the code and read it out loud. IDEs like Eclipse make renaming something like that a matter of a few simple keystrokes.

[Michael Bruce Allen]

This is SO very frustrating! Even on something this simple, I do not even understand Enums. I have now read 5 tutorials. These tutorial show days of week, month, colors, planet values... but what about comparing Enum constants to itself?

enum Choice {

     ROCK, PAPER, SISSER;

    public boolean beat(Choice choic) {
         // some sort of code I do not understand. This is almost like Inception.
    }
}

[Junilu Lacar]

The simplest thing that would possible work to make the test fail:

enum Choice {
 
     ROCK, PAPER, SCISSORS;
 
    public boolean beats(Choice choic) {
         return false;
    }
}

Spelling is "scissors"

[Junilu Lacar]

We now have four alternatives to the name for this enum: Choice, Move, Shape, Gesture. It may not seem like it would make a big difference but a well-chosen name goes a long way in making or breaking your program's readability and "flow". Also, take care of the small stuff: spell out words properly and as much as possible, choose names that make your code grammatically correct.

    ROCK.beat(SCISSORS);   // poor grammar
    ROCK.beats(SCISSORS);  // grammatically correct

[Michael Bruce Allen]

Junilu Lacar wrote:
[/code]
Spelling is "scissors"

lol. ok fair enough.

[Junilu Lacar]

Do you understand why you would write
public boolean beats(Choice choic) { return false; }
instead of a full implementation of the algorithm?

[Michael Bruce Allen]

https://github.com/CodeAmend/RockPaperScissors

Well, I started out ok I think. I took your advice and named things in a more readable fashion. I do not understand how you are not typing the name of the Enum before constants such as SCISSORS and methods such as beats. Unless somehow you made it static (if I know what I am talking about.) How did you do that?

Anyway, for the second test, it is failing and for a good reason. Now I have to learn the next step with the beats() method. Also, as far as naming goes, I wouldnt want to use Choice because it is in the java.awt.Choice. I like Gesture because it represents the pure idea of what it is.

[Michael Bruce Allen]

Here is what I got so far. I am now thinking I should have a enum for game status such as WIN, LOSE, or TIE

public enum Gesture {

    ROCK, PAPER, SCISSORS;

    public boolean beats(Gesture gesture) {
        if (this == ROCK && gesture == SCISSORS ||
            this == SCISSORS && gesture == PAPER ||
            this == PAPER && gesture == ROCK ) {
            return true;
        } else {
            return false;
        }
    }
}

public class RPS {

    // I have three valid moves: Rock, Paper, or Scissors.
    // The result will be either
    // a tie when the computer and I make the same move,
    // or I will win, or the computer will win.
    // Scissors cut Paper, Paper covers Rock, Rock smashes Scissors.

    // Winning
    @Test
    public void winningGesture() {
        Assert.assertTrue("Winning", Gesture.PAPER.beats(Gesture.ROCK));
        Assert.assertTrue("Winning", Gesture.ROCK.beats(Gesture.SCISSORS));
        Assert.assertTrue("Winning", Gesture.SCISSORS.beats(Gesture.PAPER));
    }

    //Losing
    @Test
    public void losingGesture() {
        Assert.assertFalse("Losing", Gesture.ROCK.beats(Gesture.PAPER));
        Assert.assertFalse("Losing", Gesture.PAPER.beats(Gesture.SCISSORS));
        Assert.assertFalse("Losing", Gesture.SCISSORS.beats(Gesture.ROCK));
    }

    @Test
    public void tieGesture() {
        
    }
}

[Junilu Lacar]

Michael Bruce Allen wrote:I do not understand how you are not typing the name of the Enum before constants such as SCISSORS and methods such as beats. Unless somehow you made it static (if I know what I am talking about.) How did you do that?

I use a static imports:

import static com.javaranch.general.tdd.RockPaperScissors.Gesture.ROCK;
import static com.javaranch.general.tdd.RockPaperScissors.Gesture.PAPER;
import static com.javaranch.general.tdd.RockPaperScissors.Gesture.SCISSORS;

[Michael Bruce Allen]

For your question above:
Do you understand why you would write ... instead of a full implementation of the algorithm?

I believe it is because in TDD, you want to write small incremental tests so that you do not jump ahead. So doing this caused me to make another test and it failed, causing me to question the next path to take and perhaps I might see something now that I wouldnt have if I had jumped the gun.

[Junilu Lacar]

Michael Bruce Allen wrote:Here is what I got so far. I am now thinking I should have a enum for game status such as WIN, LOSE, or TIE

public enum Gesture {

    ROCK, PAPER, SCISSORS;

    public boolean beats(Gesture gesture) {
        if (this == ROCK && gesture == SCISSORS ||
            this == SCISSORS && gesture == PAPER ||
            this == PAPER && gesture == ROCK ) {
            return true;
        } else {
            return false;
        }
    }
}

YAGNI until the test code tells you that you really need it.
In the refactoring step, you can simplify that code to this:

public enum Gesture {

    ROCK, PAPER, SCISSORS;

    public boolean beats(Gesture gesture) {
        return this == ROCK && gesture == SCISSORS ||
               this == SCISSORS && gesture == PAPER ||
               this == PAPER && gesture == ROCK;
    }
}

This is the brute force method I mentioned earlier, which is fine for now.

[Junilu Lacar]

Now, be honest, did you make sure to see your test fail before you made them pass? This is an important habit to form if you're going to do TDD. You should need to see your tests fail first. If you get comfortable seeing that green bar before you see a red bar, that's asking for trouble somewhere down the road.

[Michael Bruce Allen]

Junilu Lacar wrote:Now, be honest, did you make sure to see your test fail before you made them pass? This is an important habit to form if you're going to do TDD. You should need to see your tests fail first. If you get comfortable seeing that green bar before you see a red bar, that's asking for trouble somewhere down the road.

Well, I did start that way, but when I saw the same exact type of thing, I just wrote that out in one shot.

After I wrote ONLY "return true" next i did
https://github.com/CodeAmend/RockPaperScissors/commit/c94c276bd0c479d8d5cc48d53b386b2042712cd0
then this
https://github.com/CodeAmend/RockPaperScissors/commit/b00f19182e6be594d17fdace6579a5a256a1b83e

Maybe I jumped too fast there. I actually do not mind using a tight methodology, it is ok for me. Sometimes I just do not know how. Its not an attitude thing, I assure you, just more fault in how to think.

[Junilu Lacar]

Michael Bruce Allen wrote:

    // Winning
    @Test
    public void winningGesture() {
        Assert.assertTrue("Winning", Gesture.PAPER.beats(Gesture.ROCK));
        Assert.assertTrue("Winning", Gesture.ROCK.beats(Gesture.SCISSORS));
        Assert.assertTrue("Winning", Gesture.SCISSORS.beats(Gesture.PAPER));
    }

There's a lot of noise here. First, eliminate the need to fully qualify the Enum value by adding import statics. Add another import static for Assert.assertTrue. Then, consider what "Winning" adds to the story here. Here's my suggestion for refactoring:

    @Test
    public void winning_gestures() {
        assertTrue(ROCK.beats(SCISSORS));
        assertTrue(PAPER.beats(ROCK));
        assertTrue(SCISSORS.beats(PAPER));
    }

There is absolutely ZERO fat on that test code. I took out the comment because it's redundant with the test method name. Eliminate redundant things that make your code noisy and unnecessarily busy.

[Junilu Lacar]

With programs that have solutions as straightforward as this, it's hard to see any reason to not just jump to the final implementation. The reason for stepping through little by little when you're learning is so you can start getting into the proper mode of thinking, or "TDD groove" as I like to call it. This is what you'll miss if all you do is mindlessly follow the mechanics of the technique by rote, without awareness of the motivations and principles behind each step.

The reason for stepping in the solution is so that you can eliminate the different avenues for bugs to enter your code, one at a time. This allows you to focus on addressing each degree of variability in your program. Complex programs can have many degrees of variability and it can be overwhelming to try to program to all of them at one go. The step-by-step approach is kind of like how you'd herd cats in an open field. You put up one side of the corral first to limit movement in that direction. Then you set up another side of the corral to limit movement in two directions. Then another side of the corral, until you have enclosed the herd of cats in the corral. That's probably easier said than done when it comes to herding cats but most program solutions aren't as ornery. The point is that you want to focus on one thing at a time as much as possible.

You use the tests to set up those "walls" to corral the solution. Michael Feathers refers to these as "vices" because they lock some aspect of your program down and make it a fixed, known quantity.

Your approach of first making beats() return true was correct. That made the test pass. The next step would be to add another test or assertion to show that ROCK.beats(SCISSORS) was not handled properly by the program code. This is the first aspect of variability that you tackle. When you run the test, you'd see a red bar. Then you'd add the if (this == ROCK && gesture == SCISSORS) "vice" to lock down that aspect of behavior. Run the test again and see it pass, green bar. Then you add another test or assertion for PAPER.beats(ROCK). You get a red bar again because your program code doesn't have that "vice". You put in the vice and get a green bar. Then add the final test or assertion for SCISSORS.beats(PAPER), see the red bar, add the correct check in the program code to get a green bar.

Again, this may seem silly and tedious but as a beginner, you just want to get into that "groove". When you gain more experience and confidence, you can make bigger steps but as a beginner, making those small steps and always being conscious of the motivation behind each small step is the way you can get those "reps" in and build up your "muscle memory" for TDD.

[Junilu Lacar]

I wrote:
Your approach of first making beats() return true was correct. That made the test pass. The next step would be to add another test or assertion to show that ROCK.beats(SCISSORS) was not handled properly by the program code. This is the first aspect of variability that you tackle. When you run the test, you'd see a red bar. Then you'd add the if (this == ROCK && gesture == SCISSORS) "vice" to lock down that aspect of behavior. Run the test again and see it pass, green bar. Then you add another test or assertion for PAPER.beats(ROCK). You get a red bar again because your program code doesn't have that "vice". You put in the vice and get a green bar. Then add the final test or assertion for SCISSORS.beats(PAPER), see the red bar, add the correct check in the program code to get a green bar.

There's another way I look at the mindset here, one that goes back to the idea of a program being your "theory" of how the software should work. Initially, you add just one theory or hypothesis that says "The program will return true when asked whether the ROCK gesture beats the SCISSORS gesture." When the program initially returns false, you know it's a wrong implementation. By making the program code return true, you momentarily prove your theory. At this point it doesn't really matter that this implementation is obviously wrong. What you're after is starting the stepwise process of refinement and slowly chipping away at the wrong implementation. So you start with an implementation that is generally wrong but for that one simple case, is correct. To show that there is a part of the new implementation that is still wrong, you add a corollary theory, perhaps the converse: "The program will return false when asked whether SCISSORS beats ROCK". When the new test fails, you have proven that the program does not conform to this refinement of your theory as defined in the expanded set of tests. You then modify the program behavior again so that it does conform to the refined theory.

So it's really a cyclical process of refinement and elimination. Each time you add a test, you show that there's another bug in your program code. Then you add more implementation code to eliminate the bug that the test revealed. The TDD cycle is done when you can't think of any other tests that will show there is still at least one more bug left in the program. At this point you can say that the program's implementation conforms with the theory of how it works as embodied in the tests you have accumulated so far.

Edsger W Dijkstra, the famous computer scientist, said that "Testing shows the presence, not the absence of bugs." This is an important difference in mindset when you're doing TDD and the opposite of how tests are used by most programmers who don't do TDD.

[Junilu Lacar]

I wrote:

    @Test
    public void winning_gestures() {
        assertTrue(ROCK.beats(SCISSORS));
        assertTrue(PAPER.beats(ROCK));
        assertTrue(SCISSORS.beats(PAPER));
    }

An alternative set of tests for the above might be:

@Test
public void rock_wins() {
    assertTrue(ROCK.beats(SCISSORS));
}

@Test
public void paper_wins() {
    assertTrue(PAPER.beats(ROCK));
}

@Test
public void scissors_wins() {
    assertTrue(SCISSORS.beats(PAPER));
}

And conversely:

@Test
public void rock_loses() {
    assertFalse(ROCK.beats(PAPER));
}

@Test
public void paper_loses() {
    assertFalse(PAPER.beats(SCISSORS));
}

@Test
public void scissors_loses() {
    assertFalse(SCISSORS.beats(ROCK));
}

The end result is less important than the thought process and the steps you make to get there. One thing to note about this is that it's clearer that the programmer made very small steps in coming up with this set of tests (Edit: of course, that's with the assumption that the programmer diligently followed the TDD cycle of Red-Green-Refactor, write a little test code, write a little production code, etc.). The tests are also more focused and if they fail, you know exactly which condition caused it to fail. In contrast, the test that has multiple assertions could fail for as many different reasons as there are assertions. The more assertions you have in a test, the harder it is to pinpoint the cause for failure. This is why many TDDers prefer to have only one assertion per test method, to maximize focus and independence of each test.

[Junilu Lacar]

Another thing to note (I hope you don't get overwhelmed by all these points I'm making. Like I said, there's a lot more to think about when doing TDD than just the tests): there is a certain symmetry in the test names and the assertions being made. You should strive for symmetric code and names because it adds to the consistency of your program. The more consistent and symmetrical your program is, the easier it is to find inconsistencies and asymmetries which can harbor possible bugs or add to the cognitive load imposed on readers of the code.

[Junilu Lacar]

Michael Bruce Allen wrote:For your question above:
Do you understand why you would write ... instead of a full implementation of the algorithm?

I believe it is because in TDD, you want to write small incremental tests so that you do not jump ahead. So doing this caused me to make another test and it failed, causing me to question the next path to take and perhaps I might see something now that I wouldnt have if I had jumped the gun.

I may be reading this response wrong but a failing test during the TDD cycle should indicate progress, not a setback, and it should give you the confidence and motivation to move ahead because now you know that you have discovered another bug in your program and you know exactly why it's a bug (or at least the test should make it such that you do).

I like to tell participants in my TDD workshop that they should be happy when they see a RED BAR because it's what gives them permission to go ahead and modify the program and make it work better, and that's a Good Thing™. Going ahead without a RED BAR means that they are modifying the program code without clear justification or authorization and they do so with a higher risk of getting it wrong. The failing tests are there to guide you to the correct solution.

Edit: I guess I was reading it wrong. Yes, each test that fails should give you an opportunity to pause and consider your options for moving forward. The smaller the steps you take, the more opportunities you have to pause and consider making course corrections. It's kind of like driving: in most situations, the faster you go, the less margin of error you have to steer away from danger; the slower you go, the easier it is for you to recognize and safely avoid danger.

[Michael Bruce Allen]

Junilu Lacar wrote:Another thing to note (I hope you don't get overwhelmed by all these points I'm making. Like I said, there's a lot more to think about when doing TDD than just the tests): there is a certain symmetry in the test names and the assertions being made. You should strive for symmetric code and names because it adds to the consistency of your program. The more consistent and symmetrical your program is, the easier it is to find inconsistencies and asymmetries which can harbor possible bugs or add to the cognitive load imposed on readers of the code.

No I am not overwhelmed by these responses, in fact, I am really happy that I have the blessing of having someone like you with the heart of a teacher. I think you can see that I care though, and that is important.


Now I am at a point where I am not liking the beats() method. I feel that asks for a boolean value and I do not think that is good enough. I would rather have a different name such as attacks (of course this may be a lame name) and I came up with that because you are essentially trying to win and there are 3 possibilities: You can either WIN, LOSE, or TIE. So I was thinking make a new enum for the result of the 'attack'. But am I thinking alright, even a little bit? This would mean that this test would call for writing a new enum and also writing a new method, and perhaps that is too much. So what should I do here? I have this idea. I want to refactor, I am not sure how to go about this. Even if this idea is bad, can you just go with me here because want to feel what it is like to go with a idea and do it properly with TDD. I am going to branch off at this point and experience and ill go back to the current branch when I get a good idea on how to go about this. Thank for everything.

[Junilu Lacar]

Michael Bruce Allen wrote:
Now I am at a point where I am not liking the beats() method. I feel that asks for a boolean value and I do not think that is good enough. I would rather have a different name such as attacks (of course this may be a lame name) and I came up with that because you are essentially trying to win and there are 3 possibilities: You can either WIN, LOSE, or TIE. So I was thinking make a new enum for the result of the 'attack'. But am I thinking alright, even a little bit? This would mean that this test would call for writing a new enum and also writing a new method, and perhaps that is too much. So what should I do here? I have this idea. I want to refactor, I am not sure how to go about this. Even if this idea is bad, can you just go with me here because want to feel what it is like to go with a idea and do it properly with TDD. I am going to branch off at this point and experience and ill go back to the current branch when I get a good idea on how to go about this. Thank for everything.

Before you do that, envision the code that you would write that uses the WIN, LOSE, or TIE enum values. Then write tests that exemplify their usage. Read those tests and see if they lead you to the code you first envisioned. As a reminder, this is the code that I envisioned that led to the beats() method:

if (myMove.equals(computer)) {
    // it's a tie!
} else if (myMove.beats(computer)) {
   // I win!
} else {
   // Computer wins!
}

Do I need anything more than this? Nothing says I do so I accept the beats() API as being "Good enough" and often that's all I need to have. Being a good programmer also means that you can recognize when what you have is good enough.

[Junilu Lacar]

On the other hand, if you feel strongly about an alternative idea or you have doubts about your current implementation, by all means go ahead and experiment. It's great that you're using source control and creating branches on which to experiment with alternative solutions. That's the mark of a smart engineer and it's a good habit to develop. The only caveat I would offer is that you should be able to recognize where the cost in your time and effort in investigating the alternative starts to outweigh the potential benefits you gain from it. Don't spend too much time chasing down a better solution if there is no substantial gain from doing so.

One way to keep your enthusiasm and creativity in check is to timebox your effort. Give yourself 30 minutes or an hour to explore the alternative. If you don't arrive at a significantly better solution in that time, go back to what was good enough before.

Again, here you should be using tests as the driving force. Write tests that show how the alternative would be better than the current solution. When those tests fail, implement your alternative solution to make them pass.

[Michael Bruce Allen]

Junilu Lacar wrote:On the other hand, if you feel strongly about an alternative idea or you have doubts about your current implementation, by all means go ahead and experiment. It's great that you're using source control and creating branches on which to experiment with alternative solutions. That's the mark of a smart engineer and it's a good habit to develop. The only caveat I would offer is that you should be able to recognize where the cost in your time and effort in investigating the alternative starts to outweigh the potential benefits you gain from it. Don't spend too much time chasing down a better solution if there is no substantial gain from doing so.

One way to keep your enthusiasm and creativity in check is to timebox your effort. Give yourself 30 minutes or an hour to explore the alternative. If you don't arrive at a significantly better solution in that time, go back to what was good enough before.

Again, here you should be using tests as the driving force. Write tests that show how the alternative would be better than the current solution. When those tests fail, implement your alternative solution to make them pass.

Well, I will make a new branch and experiment just for the sake of learning new enums. But for now, the .equals was pretty good. The problem is I have the logic code inside of the enum and it returns a boolean value of true or false. This is why I liked the idea of a enum to create three states of possible scoring. I would want the Gesture enum to return something other than a boolean.

Right now, I feel the code is not good the way I did it. The logic inside of the enum Gesture is returning a boolean which is only two values and of course I am not going to return null for a tie lol. The logic you mentioned with the simple if else if else, where did you envision this code being at?

[Junilu Lacar]

Michael Bruce Allen wrote:I have this idea. I want to refactor, I am not sure how to go about this. Even if this idea is bad, can you just go with me here because want to feel what it is like to go with a idea and do it properly with TDD

Ok, I'll be yer huckleberry...

What does the high-level code that you envision uses the WIN, LOSE, TIE enums look like? Something like this perhaps?

AttackResult result = myGesture.attacks(computer);
if (result == TIE) {
   // it's a tie!
} else if (result == WIN) {
   // I win!
} else 
   // Computer wins!
}

This is plausible code. However, where is the LOSE value used? Does it make sense to use it in the end? Wouldn't that be redundant though, given that it's the only value left to consider? Is that code a lot better than the original code we envisioned?

Gesture computer = simulator.gesture();
if (myGesture.equals(computer)) {
    // it's a tie!
} else if (myGesture.beats(computer)) {
   // I win!
} else {
   // Computer wins!
}

Which version results in a simpler API? Simpler code? Which version flows better? Which one tells the story better and more succinctly? In the alternative version, how many Enums do we have? Is there any coupling between them? In the original solution, do we have more or fewer enums defined? Is there more or less coupling?

Design guideline reminder: fewer classes is generally better as this tends to result in simpler designs -- this is assuming that the classes are well-factored and adhere to design principles like SOLID, DRY, and SLAP. Fewer dependencies and less coupling is generally better because changes in one part of the program are less likely to cause a ripple effect and require changes to other parts of the program.

[Michael Bruce Allen]

I was thinking you need 3 values because your score will go up, down, or stay the same.

And no this doesnt respect solid. There are so many things to think about.. But bear with me a second, I just want to know if I need 3 values. And again, where was the logic you are mentioning located?


This logic? Where do you envision this?

AttackResult result = myGesture.attacks(computer);
if (result == TIE) {
   // it's a tie!
} else if (result == WIN) {
   // I win!
} else
   // Computer wins!
}

package com.rockpaperscissors;
import static com.rockpaperscissors.Status.*;
/**
 * Created by codeamend on 9/28/15.
 */
public enum Gesture {

    ROCK, PAPER, SCISSORS;

    public Status attacks(Gesture gesture) {
        if (this == ROCK && gesture == SCISSORS ||
            this == SCISSORS && gesture == PAPER ||
            this == PAPER && gesture == ROCK ) {
            return TIE;
        } else if(this.equals(gesture)){
            return TIE;
        } else {
            return LOSE;
        }
    }
}

I also felt I added too much too quickly but this is because this now needed to be refactored to return a enum value instead of a boolean. So I either had to delete this code and all the tests prior to this, or just change this. So I am REALLY curious, how do you handle something like this?

[Junilu Lacar]

Michael Bruce Allen wrote:The problem is I have the logic code inside of the enum and it returns a boolean value of true or false. This is why I liked the idea of a enum to create three states of possible scoring. I would want the Gesture enum to return something other than a boolean.

Right now, I feel the code is not good the way I did it. The logic inside of the enum Gesture is returning a boolean which is only two values and of course I am not going to return null for a tie lol.

Why do you feel that's bad? What design principle does this violate? What's the "smell" you're detecting?

You hint at two different responsibilities:
1. Determining whether a Gesture defeats another one.
2. Determining whether a Gesture has won, lost, or tied against another one.

The first SOLID design principle is Single Responsibility Principle. By adding WIN, LOSE, TIE and a method in Gesture that returns one of these enum values, you are actually adding the responsibility of knowing how to "judge" who the winner of a Rock Paper Scissors game is. On the other hand, the beats() method limits the responsibility of the Gesture enum to being able to compare itself to other Gestures, thus making its view of the world very small and limited only to itself. The WIN, LOSE, TIE design makes Gesture aware of and dependent on another enum, the AttackResult, or whatever you want to call it. This increases coupling and adds one dependency to your design. It's kind of a subtle nuance but it's there if you look for it.

For me, this immediately eliminates the WIN, LOSE, TIE design as a possible "better" alternative.

[Michael Bruce Allen]

AttackResult result = myGesture.attacks(computer);
Gesture computer = simulator.gesture();
if (myGesture.equals(computer)) {
    // it's a tie!
} else if (myGesture.beats(computer)) {
   // I win!
} else {
   // Computer wins!
}

Where is this logic located? I am confused by this. Where are these if statements?


I said my idea doesnt respect SOLID. I didnt know why to the detail you said, but something felt off. I would really like to finish this project. Have you seen my code?
My enum has the logic. It only returns boolean. THis is what I am griping about, how do I get it to return something else then? If I add the logic above in the MY curretnt enum, it would not be boolean.
This is why I am stuck. All my tests are written for boolean.

So if I change this, I have to rewrite all tests and how do you do this with TDD?

[Junilu Lacar]

Michael Bruce Allen wrote:And again, where was the logic you are mentioning located?

In a class that uses Gesture, in a method that encapsulates the logic for one round of the game played.

public class RockPaperScissorsGame {
    public static void main(String[] args) {
        RockPaperScissorsGame game = new RockPaperScissorsGame();
        game.play();
    }

    private void play() {
        do {
            oneTwoThreeShoot();
        } while (playAnotherRound())
    }

    private void oneTwoThreeShoot() {
        Gesture player = getPlayerGesture();
        Gesture computer = Gesture.random();
        if (player.equals(computer) {
            declareTie(player);
        } else if (player.beats(computer)) {
            declareWinner("Player", player, computer);
        } else {
            declareWinner("Computer", computer, player);
        }
    }

    // ... other methods referenced above

}

Read that code and see if you can make out a story. Is that story clear and concise? Is it well-organized? Is it broken down into logical pieces and chunks?

Edit: refactored method tied to declareTie to make it more symmetrical with declareWinner

[Junilu Lacar]

Here's how the declare* methods might be implemented:

private void declareTie(Gesture gesture) {
   System.out.printf("It's a tie! Both had %s%n", gesture);
}

private void declareWinner(String name, Gesture winner, Gesture loser) {
   System.out.printf("%s wins! %s beats %s", name, winner, loser);
}

I'll leave the other methods to you as an exercise.

[Michael Bruce Allen]

Junilu Lacar wrote:

Michael Bruce Allen wrote:And again, where was the logic you are mentioning located?

In a class that uses Gesture, in a method that encapsulates the logic for one round of the game played.

public class RockPaperScissorsGame {
    public static void main(String[] args) {
        RockPaperScissorsGame game = new RockPaperScissorsGame();
        game.play();
    }

    private void play() {
        do {
            oneTwoThreeShoot();
        } while (playAnotherRound())
    }

    private void oneTwoThreeShoot() {
        Gesture player = getPlayerGesture();
        Gesture computer = Gesture.random();
        if (player.equals(computer) {
            declareTie(player);
        } else if (player.beats(computer)) {
            declareWinner("Player", player, computer);
        } else {
            declareWinner("Computer", computer, player);
        }
    }

    // ... other methods referenced above

}

Read that code and see if you can make out a story. Is that story clear and concise? Is it well-organized? Is it broken down into logic pieces and chunks?

Edit: refactored method tied to declareTie to make it more symmetrical with declareWinner

This whole thing makes perfect sense and I am going to tell you what was throwing me off. I was just not putting the fact together that Gesture enum inherited the equals() method from Object and I was thinking it was something so different. Really Gesture had equals() inherently and we made the beats() method. I see now that all we need was a boolean. I tell you want. I absolutely love this coding stuff. It is really hard though. That is probably why I like it.

I see the whole idea now. I am going to finish and try to test my way to completion going back to the main branch.

[Junilu Lacar]

One refactoring I might perform to get to a better SLAP (Single Level of Abstraction Principle) is to extract method:

private void oneTwoThreeShoot() {
    Gesture player = getPlayerGesture();
    Gesture computer = Gesture.random();
    declareResult(player, computer);      
}

private void declareResult(Gesture player, Gesture computer) {
    if (player.equals(computer) {
        declareTie(player);
    } else if (player.beats(computer)) {
        declareWinner("Player", player, computer);
    } else {
        declareWinner("Computer", computer, player);
    }
}

This makes the code in the oneTwoThreeShoot() method have a single level of abstraction and hides the details of the "judging" process in the declareResult() method.

Edit: There is a bit of a smell that comes out with this refactoring though. The actual parameters on line 4 have the same names as the formal parameters on line 7. It's a smell because the formal parameters should be more general that the actual parameters. However, there's only one case right now where declareResult is used and that's the scenario where the game is played between a Player and the Computer. Because of this, the smell is not as repugnant. However, if you add a requirement that the program should be able to simulate play between Computer and Computer, that is, a complete simulation on both sides, then it becomes clearer that something needs to change and that the names and code in the declareResult method should be generalized accordingly. This, too, I'll leave as an exercise for you.

[Michael Bruce Allen]

Junilu Lacar wrote:One refactoring I might perform to get to a better SLAP (Single Level of Abstraction Principle) is to extract method:
This makes the code in the oneTwoThreeShoot() method have a single level of abstraction and hides the details of the "judging" process in the declareResult() method.

Yeah, that makes sense as well. I see the method now doing what it is supposed to: the action happens, the gestures get picked. Next its time for the judging...



Are these ok test? None of them failed at all. But in TDD you write failing tests right?

   @Test
    public void tieGesture() {

        Gesture myGesture;
        Gesture computer;

        myGesture = ROCK;
        computer = ROCK;
        assertTrue(myGesture.equals(computer));

        myGesture = SCISSORS;
        computer = SCISSORS;
        assertTrue(myGesture.equals(computer));

        myGesture = PAPER;
        computer = PAPER;
        assertTrue(myGesture.equals(computer));


    }

Also, the next test I would want to do is a random pick for the computer test. How do I test something that has 3 possibilities?

[Junilu Lacar]

Michael Bruce Allen wrote:
Are these ok test? None of them failed at all. But in TDD you write failing tests right?

   @Test
    public void tieGesture() {

        Gesture myGesture;
        Gesture computer;

        myGesture = ROCK;
        computer = ROCK;
        assertTrue(myGesture.equals(computer));

        myGesture = SCISSORS;
        computer = SCISSORS;
        assertTrue(myGesture.equals(computer));

        myGesture = PAPER;
        computer = PAPER;
        assertTrue(myGesture.equals(computer));
    }

Because of the nature of Enums, the behavior of equals() and == is fixed and reliable. Tests can't even guard against somebody overriding the enum equals() method because Enum has its equals() method marked as final so that's not even a possibility. The only other possible purpose a test like this could serve is as documentation of the rule that the same Gesture results in a tie. I would probably decide to skip this test altogether since it provides little to no value because the nature of enums makes testing equality of its values pretty much redundant.

[Michael Bruce Allen]

Ok, I see. Yeah, enums are special. I checked out java docs and see what you are talking about.


What about random though. How do you run a test that has a possibility of 3? It will fail 66% of the time.

[Junilu Lacar]

Michael Bruce Allen wrote:Also, the next test I would want to do is a random pick for the computer test. How do I test something that has 3 possibilities?

That's an interesting question. The math for this is probably governed by the Birthday Paradox so the number of tries you need to get to 100% certainty every time you run the test would probably be reasonably low given that you only have 3 possible values. Without doing the math, I'd guess that 500 tries would be enough to consistently get all three possible values through a pseudo-random generator. I'd probably do something like this:

	@Test
	public void random_gesture_has_three_possible_values() {
	    List<Gesture> allGestures = new ArrayList<>(Arrays.asList(ROCK, PAPER, SCISSORS));
	    for (int i = 0; !allGestures.isEmpty() && i < 500; i++) {
	        Gesture next = Gesture.random();
	        if (allGestures.contains(next)) {
	           allGestures.remove(next);
	        }
	    }
	    assertTrue(allGestures.isEmpty());
	}

To see the test fail, I'd start with a low number like 1 or 2 instead of 500 and slowly work my way up. I just ran this test about 20 times with 100 as the upper limit and got all green bars. I'm thinking that at least 500 times will make this test pass almost 100% of the time.

Edit: To make the test name be more consistent with the test assertion, I might refactor this to:

@Test
public void random_gesture_produces_all_possible_values() {
    Set<Gesture> randomGestures = new HashSet<>();
    for (int i = 0; randomGestures.size() < Gesture.values().length && i < 500; i++) {
        Gesture next = Gesture.random();
        randomGestures.add(next);
    }
    assertEquals(Gesture.values().length, randomGestures.size());
}

Using a Set rather than a List simplifies the code, too.

Yet another edit: One more refactoring to simplify (eliminate temporary variable).

@Test
public void random_gesture_produces_all_possible_values() {
    Set<Gesture> randomGestures = new HashSet<>();
    for (int i = 0; randomGestures.size() < Gesture.values().length && i < 500; i++) {
        randomGestures.add(Gesture.random());
    }
    assertEquals(Gesture.values().length, randomGestures.size());
}

There, that should be good enough.

One last edit: Ok, now I'm just showing my OCD tendencies...

Unless my assumptions are wrong and/or I plugged in the wrong parameters to the Birthday Paradox probability calculator at the bottom of this page, 50 iterations should be more than enough to guarantee a 100% pass rate for this test.

@Test
public void random_gesture_produces_all_possible_values() {
    Set<Gesture> randomGestures = new HashSet<>();
    for (int i = 0; i < 50; i++) {
        randomGestures.add(Gesture.random());
    }
    assertEquals(Gesture.values().length, randomGestures.size());
}

I might also be inclined to add some JavaDocs referencing that page and explaining how we settled on 50 iterations as the upper limit for this test.

[Michael Bruce Allen]

Junilu Lacar wrote:

Michael Bruce Allen wrote:Also, the next test I would want to do is a random pick for the computer test. How do I test something that has 3 possibilities?

That's an interesting question. The math for this is probably governed by the Birthday Paradox so the number of tries you need to get to 100% certainty every time you run the test would probably be reasonably low given that you only have 3 possible values. Without doing the math, I'd guess that 500 tries would be enough to consistently get all three possible values through a pseudo-random generator. I'd probably do something like this:

	@Test
	public void random_gesture_has_three_possible_values() {
	    List<Gesture> allGestures = new ArrayList<>(Arrays.asList(ROCK, PAPER, SCISSORS));
	    for (int i = 0; !allGestures.isEmpty() && i < 500; i++) {
	        Gesture next = Gesture.random();
	        if (allGestures.contains(next)) {
	           allGestures.remove(next);
	        }
	    }
	    assertTrue(allGestures.isEmpty());
	}

To see the test fail, I'd start with a low number like 1 or 2 instead of 500 and slowly work my way up. I just ran this test about 20 times with 100 as the upper limit and got all green bars. I'm thinking that at least 500 times will make this test pass almost 100% of the time.

Edit: To make the test name be more consistent with the test assertion, I might refactor this to:

@Test
public void random_gesture_produces_all_possible_values() {
    Set<Gesture> randomGestures = new HashSet<>();
    for (int i = 0; randomGestures.size() < Gesture.values().length && i < 500; i++) {
        Gesture next = Gesture.random();
        randomGestures.add(next);
    }
    assertEquals(Gesture.values().length, randomGestures.size());
}

Using a Set rather than a List simplifies the code, too.

Yet another edit: One more refactoring to simplify (eliminate temporary variable).

@Test
public void random_gesture_produces_all_possible_values() {
    Set<Gesture> randomGestures = new HashSet<>();
    for (int i = 0; randomGestures.size() < Gesture.values().length && i < 500; i++) {
        randomGestures.add(Gesture.random());
    }
    assertEquals(Gesture.values().length, randomGestures.size());
}

There, that should be good enough.

I really enjoyed this whole post. Before you edited I was excited about this line

 !allGestures.isEmpty() && i < 500

I did not know that you can have multiple boolean opportunities in a for constructor! Really cool.
But then I was introduced to Set and HashSet : Now this is a really cool use of code. I went ahead and made a HashSet with the enum and manually tried adding ROCK,SCISSORS and PAPER and output the size and saw that it didnt grow when I tried to add more than one of the same value in the Set. That is exactly what I thought it would do. How cool is that? I am really happy about this. I will have to learn more about these later. For now, is HashSet used mostly with Enums?

[Junilu Lacar]

Michael Bruce Allen wrote:For now, is HashSet used mostly with Enums?

No, not really. In general, a Set is a collection that doesn't allow duplicate items. If you try to add a duplicate, it gets ignored. That's why I switched from List to Set, so I could eliminate the contains() check.