0

I was making a rock paper scissors game and I'm supposed to save the last four throws of the user into a HashMap. The last four throws will be inside a Pattern class. I have it so that if the pattern is already in the HashMap, then the value will be incremented by one, showing that the user have repeated that pattern one time. The patterns will be used to predict the user next move. However, when I compare the two patterns, the one in the HashMap and the one I just passed in, even though they are not the same, it returns that they are the same. I have tried looking into this for a while but I couldn't find out what's wrong. Some help would be greatly appreciated! The error comes right at the second input. If I input R, it will save it in the HashMapbut when I input anything else, it will throw a NullPointerException, which I think because the new pattern is not stored inside the hashmap but I tried to get the value of it since the program thinks that it is equal to the one already inside the HashMap. I think the problem is inside the equals() in Pattern but I'm not entirely sure.

import java.util.*;

public class RockPaperScisors{
  public static void main(String[] args){
    Scanner key = new Scanner(System.in);
    Pattern pattern = new Pattern();
    Pattern pattern1;
    Computer comp = new Computer();
    boolean stop = false;
    int full=0;;
    while ( !stop ){

      System.out.println("Enter R P S. Enter Q to quit.");
      char a = key.next().charAt(0);
      if ( a == 'Q' ){
        stop = true;
        break;
      }
      pattern.newPattern(a);
      char[] patt = pattern.getPattern();

      for ( int i = 0; i < patt.length; i++ ){
        System.out.print(patt[i] + " ");
      }

      pattern1 = new Pattern(patt);
      comp.storePattern(pattern1);

      System.out.println();
      System.out.println("Patterns: " + comp.getSize());
      full++;
    }
  }
}

import java.util.*;

public class Pattern{
  private char[] pattern;
  private int full = 0;

  public Pattern(){
    pattern = new char[4];
  }

  public Pattern(char[] patt){
    pattern = patt;
  }

  public char[] getPattern(){
    return pattern;
  }

  public void newPattern(char p){
    if ( full <= 3 ){
      pattern[full] = p;
      full ++;
    }
    else{
      for (int i = 0; i <= pattern.length-2; i++) {
        pattern[i] = pattern[i+1];
      }
      pattern[pattern.length-1] = p;
    }
  }

  public int HashCode(){
    char[] a = pattern;
    return a.hashCode();
  }

  public boolean equals( Object o ) {
    if( o == this ) {  return true; }
    if(!(o instanceof Pattern)){ return false; }
    Pattern s = (Pattern) o;
    if (Arrays.equals(s.getPattern(), pattern))
      System.out.println("Yes");
    return Arrays.equals(s.getPattern(), pattern);
  }
}

import java.util.*;
import java.util.Map.Entry;

public class Computer{
  private HashMap<Pattern, Integer> map;

  public Computer(){
    map = new HashMap <Pattern, Integer>();
  }

  public void storePattern(Pattern p){
    boolean contains = false;
    for (Entry<Pattern, Integer> entry : map.entrySet())
    {
      Pattern patt = entry.getKey();
      if ( p.equals(patt) ){
        contains = true;
      }
    }
    if ( contains ){
      int time = map.get(p);
      time++;
      map.put(p, time);
    }
    else
      map.put(p, 0);
  }

  public int getSize(){
    return map.size();
  }
}
Improve this question
4
  • Why are you using a map to store the last 4 moves? Why not a linked list that you can just add to and remove the first element if there's more than 4 moves in it? Commented Mar 3, 2015 at 19:53
  • Because my professor requires it that way. I am aware that there are better ways to do it but we must do it this way. Commented Mar 3, 2015 at 22:02
  • Then your "professor" is bad at programming. Try to find a better teacher, or push back on his design choice by offering a far more suitable one (that's what a professional programmer would and should do). Commented Mar 3, 2015 at 22:07
  • There is also a fundamental design flaw: the map uses Pattern as its key, and if Pattern overrides equals() and hashCode() (as it should) it will be impossible to record that the same Pattern was used again. The whole design is broken. Commented Mar 3, 2015 at 22:10

3 Answers 3

Reset to default
1

Your HashCode is wrong.

It should be written in lower case.

 public int hashCode()

In order to make sure that the method is overwritten, use the @Override annotation.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

Thanks, I did not notice that!
0

As noted by another answer, the first thing to do is rename and annotate your hashcode() method.

And then, you also have to fix it. It uses 

char[] a = pattern;
return a.hashCode();

This means it uses the char[] object's hashCode() function. But that function is inherited directly from Object, and gives you a different hash code for two equal character arrays. For example, try this:

    char[] c = { 'a','b','c' };
    char[] d = { 'a','b','c' };
    System.out.printf("%d %d%n", c.hashCode(), d.hashCode());

And you'll see that it prints two different numbers.

So you need to use a better hash code function. You can make your own, or use Arrays.hashCode(pattern) (there is no need for the local a variable). The important thing is that when two Patterns are equal according to the equals() method, they should have the same hash code.

What happens in your case is that you look up the HashCode by testing equality of all the entry keys (I'll get to that in a minute, it's a bad thing to do), so equals tell you you have the same key in the hash map. But the hash map itself uses the hashCode() method in get() to locate the object. And according to the hashCode() method, there is no object in the hash map that has the same key!

So they must always agree when the objects are equal.

Now, as for your method of looking up the object:

boolean contains = false;
for (Entry<Pattern, Integer> entry : map.entrySet())
{
    Pattern patt = entry.getKey();
    if ( p.equals(patt) ){
        contains = true;
    }
}
if ( contains ){
  int time = map.get(p);
  time++;
  map.put(p, time);
} else
  map.put(p, 0);

This is not how you use a Map. The whole point of a HashMap is that you can see if it contains a certain key or not in O(1). What you are doing is iterating it and comparing - and that`s O(N), very wasteful.

If you implement your hashCode() properly, you can just look it up by doing map.containsKey(p) instead of that loop. And if you are certain that you are not putting null values in the map, you can simply use get() to get your pattern:

Integer time = map.get(p);

if ( time == null ) {
     map.put( p, 0 );
} else {
     map.put( p, time+1);
}

(You don't need to use ++, because you are not actually using time after you put it in the map).

Improve this answer

Comments

0

It's entirely possible that the issue in Pattern#HashCode.

The first issue is that it's not being used (it should be Pattern#hashCode), the second is that it's not calculating what you think it is.

You may find java.util.Arrays#hashCode very useful, changing the backing from an array to a List would also work.

As a side note, Pattern is not a great choice for the name of that class, as it clashes with java.util.regex.Pattern. This is more of a problem in this case than it might be otherwise, as it can be used with Scanner.

Improve this answer

2 Comments

Thank you for the suggestion. As for the name, I did not come up with it. The professor made us name the class Pattern. If that causes the problem I will bring it up with her in the next meeting!
Well, if your Professor chose it, you are pretty much stuck, but normally it's a best practice to avoid reusing names from the standard libraries. It's a bit odd they chose that name, but there's only so much you can do about it.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.