beginner code check

[pwolf]

recently started learning java, going through the official tutorial and would like to check that the below code is a valid and correct solution to the following exercise ( exercise 1

class Card{

    // fields -------------------------------------------------------------------------------------------------------------
    private static final String[] SUITS = {"Hearts", "Diamonds", "Spades", "Clubs"}; 
    private static final String[] RANKS = {"Ace", "2", "3", "4", "5", "6", "7", "8", "9", "10", "Jack", "Queen", "King"};

    private String mySuit;
    private String myRank; 

    private int cardSuit;
    private int cardRank;

    private boolean cardValid = false; 

    //constructors ---------------------------------------------------------------------------------------------------------
    public Card (int suit, int rank){
        cardSuit = suit;
        cardRank = rank;    
        setMyCard();    // if card valid then set values of mySuit AND myRank, else will print warning message
    }

    // methods ------------------------------------------------------------------------------------------------------------

    public String getCard(){
        String returnString = "";

        if (cardValid == true){
            String suit = getSuit();
            String rank = getRank();        
            returnString = ("Your card is the " + rank + " of " + suit + "!");
            } 

        return returnString;
    }

    public String getSuit(){
        return mySuit;
    }

    public String getRank(){
        return myRank;
    }

    public boolean isValidSuit(){
        // check suit passed to constructor is valid -- i.e 1 to 4
        if (cardSuit >=1 && cardSuit <= 4){
            return true;
        }
        else {return false;}
    }

    public boolean isValidRank(){
        // check rank passed to constructor is valid -- i.e 1 to 13
        if (cardRank >=1 && cardRank <= 13){
            return true;
        } 
        else {return false;}
    }

    public void setMyCard(){
        // if suit AND rank valid, then set mySuit AND myRank to valid string from suits[] and ranks[]
        if ( isValidSuit() && isValidRank() ){
            mySuit = SUITS[cardSuit - 1];
            myRank = RANKS[cardRank - 1];
            cardValid = true;
        } else{ System.out.println("Invalid card");}
    }

    public static void main(String[] args){

        Card newCard = new Card(3, 1);           // create new ace of spades card
        System.out.print(newCard.getCard());     // print info about card
        System.out.println();
    }

}

[rubberman]

I'm only going to say one thing - you don't validate your input data. In the constructor you do not verify that either the suit or rank passed is valid. If not, you need to throw an exception. Data validation is critical for all programming processes. If you don't verify all input data at the point of use, then you are going to have a "bad time"...

[pwolf]

True enough but the tutorial hasn't gotten to exception handling yet, so I'm not likely to do any exception handling.. my question is mainly related to the content covered in the official tutorial up to the point stated. As in, does it conform to standards, do I use the most appropriate access modifiers, and the likes. Also is it clean or should I change code layout. Thanks for your reply though, and I will be sure to start writing more robust code as I get furhter through the tutorials, and in any real exercise.

[stultuske]

there is always some refactoring you can do.

for instance:

if (cardValid == true){
// your code
}

is exactly the same as

if (cardValid){
// your code
}

the if statement expects a statement that returns either true or false as argument. a boolean already does that on itself.

if (cardSuit >=1 && cardSuit <= 4){
            return true;
        }
        else {return false;}

this can be easily refactored to:

if (cardSuit >=1 && cardSuit <= 4){
    return true;
}
return false;

since it won't reach the 'return false' statement unless it would enter the 'else' block.

This is just an example for more complicated methods. for this particular one, it can be written even simpler:

return (cardSuit >= 1 && cardSuit <= 4);

in your constructor, you are making quite a big logical error:

public Card (int suit, int rank){
        cardSuit = suit;
        cardRank = rank;    
        setMyCard();    // if card valid then set values of mySuit AND myRank, else will print warning message
    }

you are actually keeping some temporary (possible invalid) variables as instance variables. bad idea, since you hardly need them. They will, however, keep existing in your memory, even long after you've used them. so it might be better to just pass them as parameters, instead of storing them as instance variables.

also: I would declare the Card class as public

[JamesCherrill]

Ideally you should enums for Suit and Rank - but maybe you have not covered enums yet in your course.

[pwolf]

ah, of course! not sure why I missed that. I refactored the code a bit, seems cleaner now. But what makes you decide to make the Card class public?

And how would you do the constructor? In the solution from the tutorial they used the assert keyword, is that what you would do?

@JamesCherrill: The tutorial hasn't yet covered them, but I have tried to implement a couple of examples myself. However, in the exercise they state to keep the source for my solution as I will need it in the section for enums, I imagine they want to show that with enums it is easier and cleaner, so for now I avoided using them.. seemed appropriate. I've gotten too far ahead of myself and missed the point of some things in the past, now deliberatly being ignorant of anything not stated in the tutroials seems to be the way to go.

[stultuske]

the assert keyword? you mean throwing an Exception from within your constructor?
since we don't know what tutorial you are following, it's pretty hard to say anything about whether we would do the same in the same situation

[pwolf]

I provided and mentioned its from the official java tutorial from oracle. And I don't know what the assert keyword does im afraid.

[stultuske]

here they explain what assert does.

[pwolf]

• link, somehow deleted that.

thanks for the link, i'll read over it

[pwolf]

why does the following code result in the output Card@173a0067?

// fields ---------------------------------------------------------------

    private Card[][] deck = new Card[4][13];   // 4 suits , 13 ranks

    // constructors -----------------------------------------------------


    public Deck(){
        // populate deck with card objects 

        for ( int i=0; i < deck.length; i++ ){

            for ( int j=0; j < deck[i].length; j++ ){
                deck[i][j] = new Card((i + 1), (j + 1));
            }
        }

    }



public Card getCard(int suit, int rank){
        // return card from deck 
        return deck[suit-1][rank-1];
    }




public static void main(String[] args){

        Deck myDeck = new Deck();
        System.out.println(myDeck.getCard(1,1));
    }

[stultuske]

because you did not add your own implementation of the toString method.
if you don't, it 'll call the toString method that's inherited by the Object class, which returns this.

just add something like:

@Override
public String toString(){
  return mySuit + " - " + myCard;
  }

to your Card class and try again.

[pwolf]

I put the code:

    @Override
    public String toString(){
        return mySuit + " - " + myRank;
    }

in the Card class and it now prints this string when I run Deck, but I don't understand why. I also don't know what @Override does.

But from the documentation you provided, it seems whenever I want to print a string from an object I have to write a toString method like this? Im not sure I follow, I stored it in a String field and when I isntanitated objects before it always worked fine, only when I used the constructor did I get that output. Why is that?

[JamesCherrill]

When you print an object, the print or println method calls that object's toString() method to get a printable representation of that object.
toString() is defined in the Object class, so it is inherited by every other Java class. The version in Object just returns a code representing the type of object, and its hash code - technically correct info, but mostly useless. That's why you should override the inherited version, and supply your own toString() method in any new class that you create. You can ensure that your toString() returns something useful.

The @Override annotation on a method tells the compiler (and anyone reading the code) that the following method definition overrides an inherited one. Thus the compiler can check that you have overridden it correctly, and everyone else can understand what you intended.

[pwolf]

Thanks! I was just on my way to say I worked out the toString() but your info on the @Override is very helpful. The next section on teh tutorial series is on annotations, so no doubt I'll learn about it further soon enough!

I'm finding my progress ot be slow, I intend to use java to do the software section in the nand2tetris course, and then to participate with it in the next ludum dare game jam in 50 days... I also am trying to read code complete cover to cover in that time, and some other stuff. Can you offer advice for improving my rate of progress?

I'm also interested in android development, will it be easy to pick up on once I've learned java?

[stultuske]

pwolf: if you think you'll 'master' Java development in just a few weeks ... forget it.
every decent and somewhat complete (basic) course takes months, not days to complete.

a little note about the @Override annotation: though it is not mandatory to use it, it's good practice to do it anyway.

public String foString(){
  return "printable";
}

will compile and run without a problem, unless of course you try to print a value. then it'll be printing the Card@xxxx form you had earlier.

@Override
public String foString(){
  return "printable";
}

on the other hand, will not compile, since there is no foSring method in the parent class (unless you wrote that class with that method yourself).
for toString, usually it would be easy to find, but when it comes to methods with only business logic that is called somewhere in the middle of a flow, good luck bughunting;

[pwolf]

No one said master! I want to learn the syntax and cosntructs, and some of the majorly important packages, and learn what packages exist that could be useful. So that with references and documentation open, I could work on some reasonably complex software projects using Java. My goal has never been to master java, I'll forget all kinds of things by the time I learn the next! I don't have the capacity for such a thing, but it should be within my grasp to learn to use it effectively given the right tools and reference materials. I don't even know what mastering java would entail, if its knowing everything ever in detail including all libraries and tools then I doubt anyone will ever master it. If its simply knowing whats avaialble in the base langauge then surely its not such an impossible task?

Besides, I'm talking about a period of months, and focussing on it pretty much full time too.

Sorry if I got a bit defensive there!

I noticed that it wasn't required, I saw a video of it in thenewbostons playlist, but I decided not to follow his series as he had his class named using lowercase letters, which isnt the standard. I haven't read about annotations yet but I saw a video on minecraft modding and it seems you can make your own annotations? I guess in the case of custom annotations its more to help with readability?