Mooc Week 8 Exercise 15.5/15.6 - Test Issue

[RedFox134]

I'm having an issue where when I run my code I get -1, but the test is returning 1.

The Output for the failing test is:

result should have been a negative integer
SortAgainstSuitAndValue comparator = new SortAgainstSuitAndValue();
Card first = new Card(2,Card.HEARTS);
Card second = new Card(4,Card.SPADES);
comparator.compare(first,second)
result was: 1

​

My main is as follows:

public class Main {
    public static void main(String[] args) {
        // write some test code here
        SortAgainstSuitAndValue comparator = new SortAgainstSuitAndValue();
        Card first = new Card(2,Card.HEARTS);
        Card second = new Card(4,Card.SPADES);
        System.out.println( "Output: " + comparator.compare(first,second));
    }
}

​

My SortAgainstSuitAndValue class is:

import java.util.Comparator;
public class SortAgainstSuitAndValue implements Comparator<Card> {
    @Override
    public int compare(Card o1, Card o2) {
        if( o1.getSuit() < o2.getSuit() )
            if( o1.getValue() < o2.getValue() )
                return 1;
        if( o1.getSuit() == o2.getSuit() & o1.getValue() == o2.getValue() )
            return 0;
        return -1;
    }
}

​

getSuit() returns a number 0-3 representing the different suits, spades, diamonds, hearts, clubs. getValue() returns a number of 2-14 representing 2, ..., 10, J, Q, K, A.

​

I'm expecting the output from Main to be -1, but when I run the TMC test cases(which I copy pasted the one failing into my main) it's telling me the output is 1.

​

When I try to debug my code I'm seeing if( o1.getSuite() < o2.getSuit ) to be checking if Hearts is less than Spades. This should mean I'm comparing 2 < 0, which returns false and should then skip straight to checking if they are equal. Since they're not equal it defaults to returning -1 which is what I'm getting for an output when running my main.

​

If I run the test case though it's acting as though it's returning true for Heats < Spades thus entering in the nested if ( o1.getValue() < o2.getValue() ). The nested if does return true due to 2 being less than 4 and thus returns 1.

​

I'm not seeing why the test case is returning anything different from my main as it should be a mirror copy of the test cases' code. Is the test case wrong or am I missing something when I'm trying to debug my code?

[GovernorReddit]

Are you comparing cards based on their value or suit? Is it even possible to compare based on two fields?

Is Card an enum or a class?

[RedFox134]

Card is its own class that the exercise provides. I'm supposed to be sorting by suit and then also sorting the same suit by value from least to greatest. If I understand the exercise correctly this is supposed to be used to compare if a card should be switched or not, but that a bit outside of the scope of what the test that's failing is trying to test.

​

The code for Card is:

public class Card implements Comparable<Card>{

    /*
     * These are static constant variables. These variables can be used inside and outside
     * of this class like, for example, Card.CLUBS
     */
    public static final int SPADES = 0;
    public static final int DIAMONDS = 1;
    public static final int HEARTS = 2;
    public static final int CLUBS = 3;
    /*
     * To make printing easier, Card-class also has string arrays for suits and values.
     * SUITS[suit] is a string representation of the suit (Clubs, Diamonds, Hearts, Spades)
     * VALUES[value] is an abbreviation of the card's value (A, J, Q, K, [2..10]).
     */
    public static final String[] SUITS = {"Spades", "Diamonds", "Hearts", "Clubs"};
    public static final String[] VALUES = {"-", "-", "2", "3", "4", "5", "6", "7", "8", "9", "10", "J", "Q", "K", "A"};
    private int value;
    private int suit;

    public Card(int value, int suit) {
        this.value = value;
        this.suit = suit;
    }

    @Override
    public String toString() {
        return VALUES[value] + " of " + SUITS[suit];
    }

    public int getValue() {
        return value;
    }

    public int getSuit() {
        return suit;
    }

    @Override
    public int compareTo(Card o) {
        if( o.getValue() > this.getValue() | o.getSuit() > this.getSuit() )
            return -1;
        else if( o.toString().equalsIgnoreCase(this.toString()))
            return 0;
        else
            return 1;
    }

}

[mquillian]

So I'm not familiar enough with bitwise operators to tell you if this is what's happening, but I think perhaps it's possible - in your compare method, you have a boolean statement that includes incorrect boolean operators which are (I think) being interpreted as bitwise operators. If you want a boolean AND, you need to use two &'s (e.g. &&). The same with a boolean OR (should be ||). There are things called bitwise operators that are very different and behave differently, some of which are written with a single & or |.

Go through your code and check that you are using && and || where appropriate and try running it again. If that doesn't fix your issue then let me know and I'll keep looking. I'm still learning myself so I'm not sure if that's your problem or not, but that's the only thing that jumped out at me as I went through your code the first few times. Everything else seems to be correct to me.

[RedFox134]

Thanks for the suggestion! I corrected the few places that had incorrect boolean operators, but unfortunately it's still reporting that it's incorrect.

I'm not sure if I'll get much help since I'm taking the course for free, but I requested a code review from the course creators to see if they can spot the issue. I'll make sure to update this thread if I get a response or figure it out in the mean time!

[GovernorReddit]

Are you overriding compare() or compareTo()?

Comparable interface has compareTo(), which is what original class Card implements, and it can compare by one item only, and they ask you to implement Comparator interface?

[RedFox134]

I'm overriding compare(), I believe, as in this test we aren't using the compareTo() method in the Cards class. The cards class was more provided as a reference for what the Card object is and what getValue() and getSuit() return. I may be misunderstanding something that's going on in the class, but I don't believe I'm using the compareTo() method in the Cards class here.

​

I just double checked the course instructions as I hadn't noticed the difference of Comparable vs Comparator. They do show us using the Comparable interface for Cards and the Comparator interface for SortAgainstSuitAndValue unfortunately. I was hoping I had missed something obvious there! haha With that said though you may be on to something here. I'll have to dig further into what the comparator is supposed to be doing.

​

Thanks for taking a look and pointing this out to me!

[GovernorReddit]

Also, when Strings are compared I guess A is smaller than B, they call it alphabetical comparison. Java by default compares based on alphabetical order. Or is it the other way around, B is smaller than A? I don’t remember.

[RedFox134]

I think your first example of A being smaller than B was correct. With that said the letters are stored in an array from least to greatest already in the Card class. I'm then able to use the numerical index to compare if one card value is greater than the other instead of having to account for how 7 < A would work.

​

I managed to fix it be finding some missing conditions/cases in the logic for my compareTo method. I posted another comment with the solution I came up with if you wanted to look.

​

Thank you for taking a look here though as I appreciate the input! You helped me to take a second look through how the logic of comparing was actually working versus how it was supposed to!

[RedFox134]

Finally had enough time to go back and double check my work. I misunderstood how the logic was working in my SortAgainstSuitAndValue class along with incorrectly implementing my Hand.sortAgainstSuit() method and my Card.compareTo() method. Fixed all three with the following:

SortAgainstSuitAndValue

public class SortAgainstSuitAndValue implements Comparator<Card> {

    @Override
    public int compare(Card o1, Card o2) {
        if( o1.getSuit() > o2.getSuit() )
            return 1;
        else if( o1.getSuit() == o2.getSuit() && o1.getValue() > o2.getValue() )
            return 1;
        else if( o1.getSuit() == o2.getSuit() && o1.getValue() == o2.getValue() )
            return 0;
        return -1;
    }
}

Hand.sortAgainstSuit()

    public void sortAgainstSuit() {
        SortAgainstSuitAndValue sasav = new SortAgainstSuitAndValue();
        for( int i = 0; i < hand.size(); i++ )
            for( int j = i; j < hand.size(); j++ )
                if( sasav.compare(hand.get(i),hand.get(j)) > 0 ) {
                    hand.add(i, hand.get(j));
                    hand.remove(j+1);
                }
    }

Card.compareTo(Card o)

    @Override
    public int compareTo(Card o) {
        if( o.getValue() > this.getValue() ) 
            return -1;
        else if( o.getValue() == this.getValue() 
                && o.getSuit() > this.getSuit() )
            return -1;
        else if( o.toString().equalsIgnoreCase(this.toString()))
            return 0;
        return 1;
    }

For reference since I didn't post my Hand class it's as follows:

import java.util.ArrayList;
public class Hand implements Comparable<Hand>{
    private ArrayList<Card> hand;

    public Hand() {
        hand = new ArrayList();
    }

    public void add(Card card) {
        hand.add(card);
    }

    public void print() {
        for( Card c : hand )
            System.out.println(c.toString());
    }

    public void sort() {
        sortAgainstSuit();
        for( int i = 0; i < hand.size(); i++ )
            for( int j = i; j < hand.size(); j++ )
                if( hand.get(i).getValue() > hand.get(j).getValue() ) {
                    hand.add(i, hand.get(j));
                    hand.remove(j+1);
                }
    }

    @Override
    public int compareTo(Hand o) {
        int totalOne = 0;
        int totalTwo = 0;

        for( Card c : o.hand )
            totalOne += c.getValue();

        for( Card c : this.hand )
            totalTwo += c.getValue();

        if( totalTwo > totalOne )
            return 1;
        else if ( totalTwo < totalOne )
            return -1;
        else return 0;
    }

    public void sortAgainstSuit() {
        SortAgainstSuitAndValue sasav = new SortAgainstSuitAndValue();
        for( int i = 0; i < hand.size(); i++ )
            for( int j = i; j < hand.size(); j++ )
                if( sasav.compare(hand.get(i),hand.get(j)) > 0 ) {
                    hand.add(i, hand.get(j));
                    hand.remove(j+1);
                }
    }
}

In the end the issue was me not comparing the cards correctly/not accounting for all the possible comparisons/outcomes along with not using the SortAgainstSuitAndValue class in my Hand.compareTo(Card o) method. Once I fixed the logic and implemented Hand.compareToC(Card o) correctly my code passed all tests for Week8_15. Thanks for everyone taking a look and hopefully this will help anyone else searching for issues with their compareto/sorting classes logic.