Stephan van Hulst

Yes, and that's good. The problem however is that you're comparing player 1's choice nine times.

Why do you check that the player had chosen rock on line 13, when you already know they've chosen rock on line 5?

Did you try it out? Run it with all combinations of choices for player 1 and player 2 and see if it prints what you expect it to print.

Yes. So first complete the assignment in the most straightforward form: 3 conditions for the first player, with 3 x 3 conditions for the second player. Post your code when you're done, then we'll discuss a slight improvement for ties.

Z Williams wrote:For this assignment, I think this will do.

I don't think so. The point of the assignment is to eliminate the use of logic operators by nesting if-statements. The following two statements are equivalent:

if (player1.equals("rock") && player2.equals("paper")) {   // do something }
if (player1.equals("rock")) {   if (player2.equals("paper")) {     // do something   } }
Your teacher wants you to eliminate all logic operators by using nested if-statements.

What an absolutely useless exercise. Instead of teaching good programming habits, the class goes straight for the bad ones. Forget this lesson immediately when you're done with it.

First I will tell you the things that are wrong with your current code. You don't have to change it to complete the assignment, but remember them for the future. Then, I will give you a hint how you can fulfill the assignment.

You have way too much code in your main method. Methods should be short and simple, and they should do one thing well. Your main method handles both the user interface and game logic. Don't do this.

Don't use strings to represent abstract concepts like the choices a player can make in a game of Tic Tac Toe. Create separate classes to model your game.

Use appropriate names for your variables. Your variables player1 and player2 appear to represent players, but they don't. They represent choices. Name them something like firstChoice and secondChoice or player1Choice and player2Choice.

Now, here's what you can do to finish the assignment: First write three if/else clauses for each of the choices that the first player can make. Then, inside each of them, nest three if/else clauses for each of the choices that the second player can make.

Finally, you can make a slight optimization to your program by removing from each outer if or else statement the if or else statement in which the first and second player's choices are the same, and make a special case for it by checking that the two choices are equal.

There are a few things I would strongly advise to do differently:

Package names should follow the reverse host name scheme: com.coderanch.cards

Make classes final unless designed for inheritance.

Makes fields final unless designed for mutability.

Don't provide setters unless designed for mutability.

Write documentation. This is not example code, it's actually what's going to be used in a project.

Implement your methods by throwing UnsupportedOperationException, and write tests before you replace the implementation.

Validate your parameters.

Don't use string concatenation. Inside loops, use StringBuilder, and outside loops, use String.format().

Adhere to YAGNI: Why does Deck have a constant NUMBER_OF_CARDS without a clear use case for it?

Deck is a pretty useless wrapper around a list of cards. Whatever class is using a deck of cards can just maintain a collection itself. Advantage is that they can decide the appropriate collection for themselves.

Start method names with lowercase letter: shuffle().

shuffledDeck() returns an empty list.

Why is shuffledDeck() an instance method?

Be consistent in accessing your fields. Use either the field directly, or call the getter, but don't mix it up. See my preferences below.

Some personal preferences:

Nest Rank and Suit inside Card. They are strongly related to Card, and don't mean much outside of the context of a Card.

Don't prefix simple properties with get-. I find rank() and suit() less verbose, and I reserve getFoo() to express that returning a Foo involves more calculation than just returning a field.

Instead of using the getters from within the class, just use the fields directly. Less verbose and saves you a stack frame (not that it really matters).

I like to try and give equatable value types a natural order, if possible. The natural order of cards may not be applicable to all card games, but for most you will at least have a common way to sort a hand.

Inside the equals() method, I like to explicitly refer to this, and name the other instance 'other'. That way, you get code like this.rank == other.rank.

Pete Letkeman wrote:I find sometimes is nice to have as much logic as I can in private methods provided it will not be shared with other members.
This would allow private methods to be totally rewritten without changing the public method too much.

I agree using private methods is good when you need to cut up your methods in smaller pieces, but just writing a private method as an alias for a one-liner is pretty pointless.

However you may need to know about Lambdas to full advantage of them.

Functional code is great for beginners to get into as soon as possible. There are good reasons to use collections over streams, but to avoid lambda expressions is not one of them.

I suggest that you spend some time defining your classes and stubbing methods as you go. This will give you a better idea of what is needed and how it is used.

Agreed, but stub them by throwing UnsupportedOperationException.

I've had problems with long paths on Windows before, but that was when writing C# in Visual Studio. I'm not aware if it was a Windows limitation or a limitation of the libraries or the IDE. At any rate, I don't think this is a Java limitation.

Are those the real paths that cause the problem, or just an example? The examples you gave are nowhere near the limit and shouldn't cause any problems.

I suggest that once you have access to the source, you write an SSCCE to pinpoint the problem.

The problem is that you're generating a random index for all buttons, not just the buttons that haven't been selected yet.

It's also not advisable to keep track of selected button by checking the action listeners. Instead, create a proper model for your game of tic tac toe, instead of doing everything in your user interface code.

Imagine you have a Grid class that encapsulates an array of rows of marks. You can add to it a method that returns a set of unmarked positions. For a simple AI, you can then select a random position from this set.

/** * A square grid in which marks can be made. */ public final class Grid {   private final Mark[][] marks;   /**    * Creates an empty grid with the specified size.    *    * @param size the length of a side of the grid    */   public Grid(int size) {     if (size < 0)       throw new IllegalArgumentException("The size must not be negative.");     marks = new Mark[size][size];   }   /**    * Gets the size of this grid.    *    * @return the length of a side of this grid    */   public int size() {     return marks.length;   }   /**    * Validates that the specified position is inside of this grid.    *    * @throws IllegalArgumentException if position is null or any of its indices are greater than or equal to the size of this grid    */   private void validatePosition(Position position) {     if (position == null)       throw new IllegalArgumentException("The position must not be null.");     if (position.rowIndex() >= size() || position.columnIndex() >= size())       throw new IllegalArgumentException("The position must be inside of the grid.");   }   /**    * Marks the specified position in this grid.    *    * @param position the position to mark    * @param mark     the mark to place    *    * @throws IllegalArgumentException if position is null or any of its indices are greater than or equal to the size of this grid    */   void mark(Position position, Mark mark) {     validatePosition(position);     marks[position.rowIndex()][position.columnIndex()] = mark;   }   /**    * Gets the mark in the specified position in this grid.    *    * @param position the position for which to get the mark    *    * @return the mark at position, or Optional.empty() if that position is unmarked    *    * @throws IllegalArgumentException if position is null or any of its indices are greater than or equal to the size of this grid    */   public Optional<Mark> getMark(Position position) {     validatePosition(position);     return Optional.ofNullable(marks[position.rowIndex()][position.columnIndex()]);   }   /**    * Gets the unmarked positions in this grid.    *    * @return a set of all positions in this grid that are unmarked    */   public Set<Position> getUnmarkedPositions() {     Set<Position> unmarkedPositions = new LinkedHashSet<>();     for (int rowIndex = 0; rowIndex < size(); rowIndex++)       for (int columnIndex = 0; columnIndex < size(); columnIndex++)         if (marks[rowIndex][columnIndex] == null)           unmarkedPositions.add(new Position(rowIndex, columnIndex));     return unmarkedPositions;   }   /**    * Determines whether this grid is full.    *    * @return true if there are no more unmarked positions in this grid; false otherwise    */   public boolean isFull() {     for (Mark[] row: marks)       for (Mark mark: row)         if (mark == null)           return false;     return true;   }   /**    * Gets the positions of all marks that form a streak across this grid, if such a streak exists.    *    * @return a list of positions that all contain the same mark and form either a horizontal, vertical or diagonal line across the entire grid, or Optional.empty() if no such streak exists    */   public Optional<List<Position>> getStreak() {     throw new UnsupportedOperationException("Not implemented yet.");   } }

Furthermore, can you post the Maven output?

Two questions. Why are you using a random index, and why are you checking for listeners in the first place?

In Dutch the words are 'wijdte' and 'breedte'. They are interchangeable, but I think 'breedte' is used more often.

Well, what do you think that line 5 means, if it were valid?

Yes, but only if the first argument of the functional interface is of the type that you want to apply the method reference to.

You might find it helpful to rewrite method declarations and functional interfaces as a canonical type expression. I use a form of lambda calculus that is prevalent in languages like Haskell.

BiPredicate<Integer, Integer> represents a function that takes two integers and returns a boolean. You can write it as Integer -> Integer -> boolean.

MyClass.evaluate() is a function that takes two integers and returns a boolean, but it also takes a MyClass because it's an instance method. You can write it as MyClass -> Integer -> Integer -> boolean.

Method handles can be written as an operation on a class name, or as an operation on an object reference. When you operate on the class name, you get the full type expression of that method, so MyClass::evaluate is a MyClass -> Integer -> Integer -> boolean. As you can see, this is not compatible with the BiPredicate.

When you operate on an object reference, the type expression is the same, except you omit the first operand. obj::evaluate is an Integer -> Integer -> boolean. This is compatible with the BiPredicate.

You can juggle around with this. When a higher order function accepts an X -> Y -> Z, you can pass it all of the following handles:

X::instance_Method_Accepting___Y_Returning_Z
W::static___Method_Accepting__XY_Returning_Z
w::instance_Method_Accepting_WXY_Returning_Z

Welcome to CodeRanch!

BiPredicate<Integer, Integer> represents a function that takes two integers and returns a boolean. MyClass.evaluate(int, int) is a function that takes a MyClass and two integers, and returns a boolean, so it doesn't fit.

You can still make it compile using a method reference though, by supplying an object to the method reference:
MyClass obj = new MyClass(); biPredicate = obj::evaluate;