-1

I have a problem in a class, Shipping, in my monopoly game. I want to find out how many shippings a player owns and thereby determine the rent. (It is a danish version of the game, so i think the rules differ from country to country). I use the Collections.frequency but for some reason it always goes to the else statement and gives me 8 times the rent. Here is the code:

package fields;

import java.util.Collections;
import java.util.Scanner;

public class Shipping extends Ownable
{
    public int rent;
    private int occurence;
    private String yesOrNo;
    private Shipping shipping;
    Scanner scan = new Scanner(System.in);

public Shipping(int fieldNumber, String fieldName, int price)
{
    super(fieldNumber, fieldName, price);
    this.rent = 500;
}

public int getRent()
{
    return rent;
}

@Override
public int getPrice()
{
    return price;
}

@Override
public String getName()
{
    return fieldName;
}

@Override
public int getNumber()
{
    return fieldNumber;
}

@Override
public matador.Player getOwner()
{
    return owner;
}

@Override
public void getConsequence()
{
    if(getOwner() == null)
    {
        System.out.println("Do you want to buy it? (Y/N)");
        yesOrNo = scan.next();

        if(yesOrNo.equalsIgnoreCase("y"))
        {
            matador.Main.currentPlayer.money -= getPrice();
            owner = matador.Main.currentPlayer;
            matador.Main.currentPlayer.ownedGrounds.add((Shipping) data.board[matador.Main.currentPlayer.getPosition()]);
            System.out.println(matador.Main.currentPlayer.getName()
                    + " bought " + getName());
        }
        else
        {
            System.out.println(matador.Main.currentPlayer.getName() +
                    " did not buy " + data.board[matador.Main.currentPlayer.getPosition()].getName());
        }
    }
    else if(getOwner() != matador.Main.currentPlayer)
    {
        occurence = Collections.frequency(owner.ownedGrounds, shipping);
        if(occurence == 1)
        {
            System.out.println(matador.Main.currentPlayer.getName() + " needs to pay " +
                    owner.getName() + " " + getRent());
            matador.Main.currentPlayer.money -= getRent();
            owner.money += getRent();
        }
        else if(occurence == 2)
        {
            System.out.println(matador.Main.currentPlayer.getName() + " needs to pay " +
                    owner.getName() + " " + (2 * getRent()));
            matador.Main.currentPlayer.money -= (2 * getRent());
            owner.money += (2 * getRent());
        }
        else if(occurence == 3)
        {
            System.out.println(matador.Main.currentPlayer.getName() + " needs to pay " +
                    owner.getName() + " " + (4 * getRent()));
            matador.Main.currentPlayer.money -= (4 * getRent());
            owner.money += (4 * getRent());
        }
        else
        {
            System.out.println(matador.Main.currentPlayer.getName() + " needs to pay " +
                    owner.getName() + " " + (8 * getRent()));
            matador.Main.currentPlayer.money -= (8 * getRent());
            owner.money += (8 * getRent());
        }
    }
}
}

I think the problem is the object i try to get the frequency of, but i have no idea.

thanks in advance!

Regards!

  • Have you tried to debug? Set a breakpoint and have a look inside your collection – Uwe Allner Nov 10 '14 at 08:31
  • The _frequency_ is most probable 0. So it is neither 1, nor 2, nor 3, and thus the else branch is taken. The reason might be that you didn't provide a proper `equals` method (and thus also a proper `hashCode` method) in your `Shipping` class. – Seelenvirtuose Nov 10 '14 at 08:33
  • just a short codereview: the code in the if/elses is identical. extract it into a method, with the number (1,2,4,8 * getRent()) and the Player and Owner as arguments – Joeblade Nov 10 '14 at 09:39

2 Answers2

0
occurence = Collections.frequency(owner.ownedGrounds, shipping);

Collections.frequency relies on the object's equals method to compare the objects in the Collection to the object whose occurrences you wish to find. Since your Shipping class doesn't seem to override equals, is uses Object's default implementation, which compares the references.

Therefore you will always get a frequency of 0 (unless you pass a Shipping reference that is already in the Collection, in which case you would get a frequency of 1, but it would probably still be the wrong result).

Eran
  • 387,369
  • 54
  • 702
  • 768
0

Collections.frequency(c, o) uses the equals() method for comparison. I can see that you've not overridden it with your own implementation, therefore it will default to Object.equals() which uses references.

Although not strictly needed for this problem you should also implement .hashCode() when implementing equals().

This is stated sort-of in the javadoc.

Returns the number of elements in the specified collection equal to the specified object. More formally, returns the number of elements e in the collection such that (o == null ? e == null : o.equals(e)).

Community
  • 1
  • 1
Adam
  • 35,919
  • 9
  • 100
  • 137