Code review Java am i on the right track?

[themarval]

I've searched the page but i can't find a way to mark my previous post as closed. This is slightly different so i was not sure if i should create a new post or add to my existing one. Not trying to break any rules intentionally.

in this post i've created a helper and a main class. This program can generate 5 random cards and display the results as a text screen. I've managed to get this output to appear in a Jpanel as 5 separate sub panels. Right now I have static card images in the sub panels as well so that i can keep my bearings. I still can not figure out how to link my image to my output. I'm not looking for anyone to finish my homework for me, but i am looking to see if 1) my code is correct and 2) what process do i need to code in order to make my images match my output. This isnt pretty code, but i'm still learning

import java.awt.*;
import javax.swing.*;
public class Card


{
	//define class variables

   private int face;
	private int suit; 
	
	private String faceName = "Ace, King, Queen, Jack, Ten, Nine, Eight, Seven, Six, Five, Four, Three, Two";
	private String suitName = "Heart, Diamond, Spade, Club";
	private String fullName = "King Heart, King Diamond, King Spade, King Club,Queen Heart, Queen Diamond, Queen Spade," +
	"Queen Club, Jack Heart, Jack Diamond, Jack Spade, Jack Club, Ten Heart, Ten Diamond, Ten Spade, Ten Club, Nine Heart,"+
	"Nine Diamond, Nine Spade, Nine Club, Eight Heart, Eight Diamond, Eight Spade, Eight Club, Seven Heart, Seven Diamond,"+ 
	"Seven Spade, Seven Club, Six Heart, Six Diamond, Six Spade, Six Club,	Five Heart, Five Diamond, Five Spade, Five Club,"+
	"Four Heart, Four Diamond, Four Spade, Four Club, Three Heart, Three Diamond,	Three Spade, Three Club, Two Heart,"+
	"Two Diamond, Two Spade, Two Club, Ace Heart, Ace Diamond, Ace Spade, Ace Club";
	
	
	
   ImageIcon c1icon = new ImageIcon ("clubs-2-75.jpg");
	ImageIcon c2icon = new ImageIcon ("clubs-3-75.jpg");
	ImageIcon c3icon = new ImageIcon ("clubs-4-75.jpg");
	ImageIcon c4icon = new ImageIcon ("clubs-5-75.jpg");
	ImageIcon c5icon = new ImageIcon ("clubs-6-75.jpg");
	ImageIcon c6icon = new ImageIcon ("clubs-7-75.jpg");
	ImageIcon c7icon = new ImageIcon ("clubs-8-75.jpg");
	ImageIcon c8icon = new ImageIcon ("clubs-9-75.jpg");
	ImageIcon c9icon = new ImageIcon ("clubs-10-75.jpg");
	ImageIcon c10icon = new ImageIcon ("clubs-a-75.jpg");
	ImageIcon c11icon = new ImageIcon ("clubs-j-75.jpg");
	ImageIcon c12icon = new ImageIcon ("clubs-k-75.jpg");
	ImageIcon c13icon = new ImageIcon ("clubs-q-75.jpg");
	ImageIcon c14icon = new ImageIcon ("diamonds-2-75.jpg");
	ImageIcon c15icon = new ImageIcon ("diamonds-3-75.jpg");
	ImageIcon c16icon = new ImageIcon ("diamonds-4-75.jpg");
	ImageIcon c17icon = new ImageIcon ("diamonds-5-75.jpg");
	ImageIcon c18icon = new ImageIcon ("diamonds-6-75.jpg");
	ImageIcon c19icon = new ImageIcon ("diamonds-7-75.jpg");
	ImageIcon c20icon = new ImageIcon ("diamonds-8-75.jpg");
	ImageIcon c21icon = new ImageIcon ("diamonds-9-75.jpg");
	ImageIcon c22icon = new ImageIcon ("diamonds-10-75.jpg");
	ImageIcon c23icon = new ImageIcon ("diamonds-a-75.jpg");
	ImageIcon c24icon = new ImageIcon ("diamonds-j-75.jpg");
	ImageIcon c25icon = new ImageIcon ("diamonds-k-75.jpg");
	ImageIcon c26icon = new ImageIcon ("diamonds-q-75.jpg");
	ImageIcon c27icon = new ImageIcon ("hearts-2-75.jpg");
	ImageIcon c28icon = new ImageIcon ("hearts-3-75.jpg");
	ImageIcon c29icon = new ImageIcon ("hearts-4-75.jpg");
	ImageIcon c30icon = new ImageIcon ("hearts-5-75.jpg");
	ImageIcon c31icon = new ImageIcon ("hearts-6-75.jpg");
	ImageIcon c32icon = new ImageIcon ("hearts-7-75.jpg");
	ImageIcon c33icon = new ImageIcon ("hearts-8-75.jpg");
	ImageIcon c34icon = new ImageIcon ("hearts-9-75.jpg");		
	ImageIcon c35icon = new ImageIcon ("hearts-10-75.jpg");		
	ImageIcon c36icon = new ImageIcon ("hearts-a-75.jpg");	
	ImageIcon c37icon = new ImageIcon ("hearts-j-75.jpg");	
	ImageIcon c38icon = new ImageIcon ("hearts-k-75.jpg");	
	ImageIcon c39icon = new ImageIcon ("hearts-q-75.jpg");	
	ImageIcon c40icon = new ImageIcon ("spades-2-75.jpg");
	ImageIcon c41icon = new ImageIcon ("spades-3-75.jpg");
	ImageIcon c42icon = new ImageIcon ("spades-4-75.jpg");
	ImageIcon c43icon = new ImageIcon ("spades-5-75.jpg");
	ImageIcon c44icon = new ImageIcon ("spades-6-75.jpg");
	ImageIcon c45icon = new ImageIcon ("spades-7-75.jpg");
	ImageIcon c46icon = new ImageIcon ("spades-8-75.jpg");
	ImageIcon c47icon = new ImageIcon ("spades-9-75.jpg");		
	ImageIcon c48icon = new ImageIcon ("spades-10-75.jpg");		
	ImageIcon c49icon = new ImageIcon ("spades-a-75.jpg");	
	ImageIcon c50icon = new ImageIcon ("spades-j-75.jpg");	
	ImageIcon c51icon = new ImageIcon ("spades-k-75.jpg");	
	ImageIcon c52icon = new ImageIcon ("spades-q-75.jpg");	
	

   public Card() 
   {
	
      face = (int) (Math.random() * 13 + 1);      //create a random value for the face values
		suit = (int) (Math.random() *4 + 1);			 //create a random value for the suit values

		
		setFaceName();
		setSuitName();

			
   }

   private void setFaceName()  
   {
	
      switch (face)
		{
			case 1: faceName = "Ace"; break;				
			case 2: faceName = "Two"; break;
			case 3: faceName = "Three"; break;
			case 4: faceName = "Four"; break;
			case 5: faceName = "Five"; break;		
			case 6: faceName = "Six"; break;		
			case 7: faceName = "Seven"; break;		
			case 8: faceName = "Eight"; break;		
			case 9: faceName = "Nine"; break;		
			case 10: faceName = "Ten"; break;		
			case 11: faceName = "Jack"; break;					
			case 12: faceName = "Queen"; break;		
			case 13: faceName = "King"; break;		
		}

   }

   public void setSuitName()
	
   {
      switch (suit)
		
		{
			case 1: suitName = "Club"; break;
 			case 2: suitName = "Diamond"; break;
 			case 3: suitName = "Spade"; break;
 			case 4: suitName = "Heart"; break;
 
   }
}



   public int getFace ()
	{
			return face;
	}
	
   public int getSuit()
   {

				return suit;
   }

  
  
   public String toString()
   {

				String result =faceName + " " + suitName;
				return  result;
				
				
	}	 
 }

import java.awt.*;
import javax.swing.*;

public class FrameCardimage

{
  
	
   public static void main (String[] args)
   {

 // Create card objects c1, c2, c3, c4 and c5
       Card c1,c2, c3, c4,c5;
    
	    c1 = new Card ();
		 c2 = new Card ();
		 c3 = new Card ();
		 c4 = new Card ();
		 c5 = new Card ();
   
      JFrame frame = new JFrame ("Any 5 Cards");
      frame.setDefaultCloseOperation (JFrame.EXIT_ON_CLOSE);

ImageIcon pic1 = new ImageIcon ("spades-2-75.png");
ImageIcon pic2 = new ImageIcon ("hearts-2-75.png");
ImageIcon pic3 = new ImageIcon ("diamonds-2-75.png");
ImageIcon pic4 = new ImageIcon ("spades-2-75.png");
ImageIcon pic5 = new ImageIcon ("spades-4-75.png");

 JLabel label1, label2, label3, label4, label5, label6, label7, label8, label9, label10;


      // Create first subpanel
      JPanel subPanel1 = new JPanel();
      subPanel1.setBackground (Color.cyan);
		label1 = new JLabel (pic1);
		label2 = new JLabel (c1 + " - (" + c1.getFace() + ", " + c1.getSuit() + ")");
		subPanel1.add (label1);  
		subPanel1.add (label2);


      // Creaet second subpanel.  PreferredSize not needed, let display default as needed.
		
      JPanel subPanel2 = new JPanel();
      subPanel2.setBackground (Color.cyan);
		label3 = new JLabel (pic2);
 		label4 = new JLabel (c2 + " - (" + c1.getFace() + ", " + c1.getSuit() + ")");
		subPanel2.add (label3);
		subPanel2.add (label4);


		JPanel subPanel3 = new JPanel();
      subPanel3.setBackground (Color.cyan);
		label5 = new JLabel (pic3);
		label6 = new JLabel (c3 + " - (" + c1.getFace() + ", " + c1.getSuit() + ")");
		subPanel3.add (label5);
		subPanel3.add (label6);


		JPanel subPanel4 = new JPanel();
      subPanel4.setBackground (Color.cyan);
		label7 = new JLabel (pic4);
		label8 = new JLabel (c4 + " - (" + c1.getFace() + ", " + c1.getSuit() + ")");
		subPanel4.add (label7);
		subPanel4.add (label8);


		JPanel subPanel5 = new JPanel();
	   subPanel5.setBackground (Color.cyan);
		label9 = new JLabel (pic5);
		label10 = new JLabel (c5 + " - (" + c1.getFace() + ", " + c1.getSuit() + ")");
		subPanel5.add (label9);
		subPanel5.add (label10);

      // Set up primary panel
      JPanel primary = new JPanel();
 		primary.setPreferredSize (new Dimension (1000, 500));  
      primary.setBackground (Color.yellow);
		primary.add (subPanel1);
		primary.add (subPanel2);
		primary.add (subPanel3);
		primary.add (subPanel4);
		primary.add (subPanel5);

      frame.getContentPane().add(primary);
      frame.pack();
      frame.setVisible(true);
   }
}

[mrnutty]

Why don't you use arrays ? For example his code :

ImageIcon c1icon = new ImageIcon ("clubs-2-75.jpg");
	ImageIcon c2icon = new ImageIcon ("clubs-3-75.jpg");
	ImageIcon c3icon = new ImageIcon ("clubs-4-75.jpg");
	ImageIcon c4icon = new ImageIcon ("clubs-5-75.jpg");
	ImageIcon c5icon = new ImageIcon ("clubs-6-75.jpg");
	ImageIcon c6icon = new ImageIcon ("clubs-7-75.jpg");
	ImageIcon c7icon = new ImageIcon ("clubs-8-75.jpg");
	ImageIcon c8icon = new ImageIcon ("clubs-9-75.jpg");
	ImageIcon c9icon = new ImageIcon ("clubs-10-75.jpg");

Can be substituted with this :

ImageIcon[]  clubSuitIcon = new ImageIcon[10-2]
for(int i  = 0; i < clubSuitIcon.length; ++i){
  String card= "clubs-" + String.valueOf(i + 2) + "-75.jpg";
  clubSuitIcon[i] = new ImageIcon(card);
}

Making similar changes can make your code much better.

[~s.o.s~]

Instead of using Strings, use enums for your "face" and "suit" types. Create a separate class Card which would contain the card related information like face, suit, representation etc.

Always think about abstractions and layers when creating an application. The "Card" type (Model) can be used for creating a swing application, a web application, an andriod application (different views). With the current program design, you have little scope of re-use. It's leaky abstractions all the way. Read more about the MVC design pattern. When designing OO software, think of ways of how you can push responsibilities.

Of course, all this need not happen immediately. Take your time, understand why this program is not flexible, think of how you can reduce the complexity etc. If you can afford a book, I'd recommend Head First OOAD as a good starting point. Another interesting read might be; never do any work that you can get someone else to do for you.

Good luck :)

[themarval]

Wow - this is definately much cleaner! I haven't learned how to work with arrays yet, so if you don't mind i'd just like to make sure that I understand what you're presenting to me.
Line 1 of your code creates an object called "clubSuitIcon" giving it a parameter range value of 10 - 2. Does the order of this parameter matter? I'm curious as to why it would not be 2-10.
Line 2 initializes the counter at zero and says if the clubSuitIcon length is less than i, increase the counter by one. Not sure if i get why we are using the .length method? Line 3 says if the returned string value is "clubs" then increase the counter by 2 and concatenate the specific file. ??? I can tell that here we're telling the code what jpg file to pull but I don't know how to translate the String.valueOf(i+2) ? Line 4 we're assigning the card value to the clubSuitIcon object.

In using this code i would not have to declare each individual .jpg file. This would need to go into the helper file. But i would need to create a separate method for each suit? Or can i create one function called getSuitIcon that would have this code repeated for each suit?

then in my panel code i could modify

label10 = new JLabel (c5 + " - (" + c1.getFace() + ", " + c1.getSuit() + ")");

to read

label10 = new JLabel (c5 + " - (" + c1.getFace() + ", " + c1.getSuit() + c1.getSuitIcon() +")");

ImageIcon[]  clubSuitIcon = new ImageIcon[10-2]
for(int i  = 0; i < clubSuitIcon.length; ++i){
  String card= "clubs-" + String.valueOf(i + 2) + "-75.jpg";
  clubSuitIcon[i] = new ImageIcon(card);
}

Why don't you use arrays ? For example his code :

ImageIcon c1icon = new ImageIcon ("clubs-2-75.jpg");
	ImageIcon c2icon = new ImageIcon ("clubs-3-75.jpg");
	ImageIcon c3icon = new ImageIcon ("clubs-4-75.jpg");
	ImageIcon c4icon = new ImageIcon ("clubs-5-75.jpg");
	ImageIcon c5icon = new ImageIcon ("clubs-6-75.jpg");
	ImageIcon c6icon = new ImageIcon ("clubs-7-75.jpg");
	ImageIcon c7icon = new ImageIcon ("clubs-8-75.jpg");
	ImageIcon c8icon = new ImageIcon ("clubs-9-75.jpg");
	ImageIcon c9icon = new ImageIcon ("clubs-10-75.jpg");

Can be substituted with this :

ImageIcon[]  clubSuitIcon = new ImageIcon[10-2]
for(int i  = 0; i < clubSuitIcon.length; ++i){
  String card= "clubs-" + String.valueOf(i + 2) + "-75.jpg";
  clubSuitIcon[i] = new ImageIcon(card);
}

Making similar changes can make your code much better.

[mrnutty]

>>Line 1 of your code creates an object called "clubSuitIcon" giving it a parameter range value of 10 - 2. Does the order of this parameter matter? I'm curious as to why it would not be 2-10.

I wrote " ImageIcon[] clubSuitIcon = new ImageIcon[10-2] "
because I though it would show you that there are 10 - 2 = 8 elements.
I guess it would have been better if I did :
" ImageIcon[] clubSuitIcon = new ImageIcon[8] ".

Since your cards start from 2( of clubs) and end with the number(10 of clubs), the "10 - 2" was supposed to show you

>>that there are 10 - 2 = 8
cards to represent. The rest are J,Q,K, A.

>>Line 2 initializes the counter at zero and says if the clubSuitIcon length is less than i, increase the counter by one.

Its the other way around. If "i" is less than "the array length"
then increment "i" by one.

>>Not sure if i get why we are using the .length method?

.length is not a method. Its a constant variable. We use it
so that we don't access an element pass the array.

>>Line 3 says if the returned string value is "clubs" then increase the counter by 2 and concatenate the specific file. ??? I can tell that here we're telling the code what jpg file to pull but I don't know how to translate the String.valueOf(i+2)

Line 3 reads as follows :

A string variable called card equals the string "clubs-"
concatenated with the string representation of the value of (i+2)
concatenated withe the string "-75.jpeg".

The main problem you probably have is the notion of concatenation. A String concatenated with another String represents 1 String that "adds" both of the string together.
For example :

String str1 = "hello";
String str2 = "world";
String message = str1 + str2 + " from java."

The String message now equals "hello world from java". As you see the concatenation is just like attaching the 2 String together
from left to right.