information about my code

[felito]

hi guys

i have this code

public class Data {
	private JLabel[] lab;
	private JTextField pn10;
	private JLabel[] putImage;
	private JTextField[] pnCardNumber;
	private JTextField[] pnCardSuit;
	private JTextField[] pnCardName;
	private int gameNumber;
	private JButton[][] arrayButtons;
	private PlayHand playHand;
	private Player[] player;
	private JTextField order;
	private JTextField[] text;

	//above action listener for all buttons

}

in this case i create setters and getters for all variables, and because of that my code have more 200 lines, my question is if i put all as public it is incorrect?

another question

i need to pass this parameters to another class

Data(JLabel[] lab, JTextField pn10, JLabel[] test,
			JTextField[] pnCardNumber, JTextField[] pnCardSuit,
			JTextField[] pnCardName, int gameNumber, JButton[][] arrayButtons,
			PlayHand playHand, Player[] player, JTextField order,
			JTextField[] text) {
		this.setLab(lab);
		this.setPn10(pn10);
		this.setPutImage(test);
		this.setPnCardNumber(pnCardNumber);
		this.setPnCardSuit(pnCardSuit);
		this.setPnCardName(pnCardName);
		this.gameNumber = gameNumber;
		this.setArrayButtons(arrayButtons);
		this.playHand = playHand;
		this.player = player;
		this.setOrder(order);
		this.setText(text);
	}

so...much code. But it's works, it is bad do that?

Thanks for any help

[Junky]

my question is if i put all as public it is incorrect?

It is neither correct nor incorrect. They are just two different ways to implement the code. However, making variables private and limiting access through methods allows you greater control. Consider:

class Foo {
    public int postiveNumber; // variable to hold a number greater than 0
}

Foo f = new Foo();
f.positiveNumber = -100; // Hmmmm!

so...much code. But it's works, it is bad do that?

Once again it depends. Do all those labels, textfields etc reside in another class? Perhaps you could pass an instance of that class instead. Or myabe you should google the MVC pattern.

[al_Marshy_1981]

What is the class Data main purpose? Are you making it do anything that conflicts with that purpose, can you move some of the things that Data is currently doing that conflicts with its purpose into another class that would make more sense. These are some but certainly not all of the questions you should be asking when designing/reviewing your Data class or any class.

If memeber fields are starting to get big it can be a sign that you are making your class try to be many things instead of representing one thing and one thing only very well.

Keep member data private by default is a good rule of thumb.

[felito]

yes i can instantiate a class and remove all code, but the problem is that i have a method that create a new game i receive nullPoiter exception because the method is inside of the class where i remove all attributes.

Do you understand the problem that i tell you?

[Junky]

No. Post some relevant code.

[felito]

i have two classes.

SWING and data of swing.

In the past i pass all parameters, as i post in the beginning.

Now i instantiate the class of data in the swing, and remove all the code of parameters (that i post initially)

so, my class of data have this method

public void newGame(int number) {

		final Deck deck = new Deck();
		gameNumber = number;

		// shuffle the deck
		deck.shuffle();

		//Name of the game
		if (gameNumber == 1) {
			getOrder().setText("1st hand - No tricks");
		}
		else if (gameNumber == 2) {
			getOrder().setText("2nd hand - No Hearts");
		}
		else 
			getOrder().setText("3rd hand - No Queens");

		// Create all Players
		for (int i = 0; i < player.length; i++) {
			player[i] = new Player();
		}

		// give cards to four players
		for (int i = 0; i < 13; i++) {
			for (int t = 0; t < player.length; t++) {
				player[t].receiveCard(deck.pickNextCard());
			}

		}

		// put cards on buttons
		for (int i = 0; i < getArrayButtons().length; i++) {
			for (int j = 0; j < getArrayButtons()[i].length; j++) {
				getArrayButtons()[i][j].setText(player[i].getCard(j).toString());
				getArrayButtons()[i][j].setVisible(true);
			}
		}

		int hands = 0;
		int firstplayer = 0;
		int cardsEnabled = 0;

		// number of rounds of each game, change number to 13 if you want a
		// complete round. Now each game have three rounds for more easy debugging
		while (hands < 3) {

			if (playHand.getNumberOfCards() < player.length) {

				try {
					Thread.sleep(30);
				} catch (InterruptedException e) {
					// TODO Auto-generated catch block
					e.printStackTrace();
				}
				if (cardsEnabled <= playHand.getNumberOfCards()) {
					setPlayer((firstplayer + playHand.getNumberOfCards()) % 4);
					cardsEnabled++;
				}
			}

			if (playHand.getNumberOfCards() == 4) {
				int winner = playHand.winner();
				int value = playHand.scoreFirstGame();

				//score
				if (winner == 0) {
					getText()[0].setText("Player 1 winner: "+Integer.toString(value) + " negative");
				}
				else if (winner == 1) {
					getText()[1].setText("Player 2 winner: "+Integer.toString(value) + " negative");
				}
				else if (winner == 2) {
					getText()[2].setText("Player 3 winner:  "+Integer.toString(value) + " negative");
				}
				else if (winner == 3) {
					getText()[3].setText("Player 4 winner: "+Integer.toString(value) + " negative");
				}

				try {
					Thread.sleep(2000);
				} catch (InterruptedException e) {
					// TODO Auto-generated catch block
					e.printStackTrace();
				}
				
				playHand.clearHand();
				for (int k = 0; k < getPutImage().length; k++) {
					getPutImage()[k].setIcon(null);
				}

				hands++;
				firstplayer = winner;
				cardsEnabled = 0;
			}
			number++;
		}
	}

off course, that is a null pointer exception when i call this method in the main class.

[Junky]

Post your main method and copy and paste the EXACT error message.

[felito]

main method and error:

package game;

public class Game {

	/**
	 * The main method.
	 * 
	 * @param args
	 *            the arguments
	 */
	public static void main(String[] args) {
		// TODO Auto-generated method stub
		KingData king = new KingData();

		// Three games
		king.newGame(1);
		king.newGame(2);
		king.newGame(3);

	}

}

Exception in thread "main" java.lang.NullPointerException
	at game.KingData.newGame(KingData.java:114)
	at game.Game.main(Game.java:16)

getOrder().setText("1st hand - No tricks"); // first line error - 114

[al_Marshy_1981]

You need an instance of Swing(with accessor methods to all its data you need like Player etc) in your KingData class. That is how it looks to me anyway.

[Junky]

It's like pulling teeth so I am making some assumptions. Let's say that you have a class called GUIStuff with all the textfields, labels etc.

// main method
GUIStuff gs = new GUIStuff();
KingData king = new KingData(gs);

// KingData class
GUIStuff gooey;

KingData(GUIStuff g) {
    gooey = g;
}

gooey.getOrder().setText("1st hand - No tricks");

This makes another assumption that getOrder has been initialised correctly in the GUIStuff class.

[felito]

but if i did an instance of kindata in king and a instance of king in kingdata, i have an infinite loop

[Junky]

Gah!

I just realised getOrder is a method. None of this makes sense as you are not providing us with enough and relevant data. For example you start of with a Data class. Later you say you have a Swing class (is that its name or fo you mean it has all the swing stuff in it?). Then Finally you try creating a KingData object. Yet another class.

For the love of my sanity get your facts straight.

[al_Marshy_1981]

Post everything you have, try and rename classes if possible to more descriptive titles than king and swing.

[felito]

ok

KingData

package game;

import java.awt.Color;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.JButton;
import javax.swing.JLabel;
import javax.swing.JTextField;

public class KingData {
	private JLabel[] lab;
	private JTextField pn10;
	private JLabel[] putImage;
	private JTextField[] pnCardNumber;
	private JTextField[] pnCardSuit;
	private JTextField[] pnCardName;
	private int gameNumber;
	private JButton[][] arrayButtons;
	private PlayHand playHand;
	private Player[] player;
	private JTextField order;
	private JTextField[] text;

	// action listener for all buttons
	public ActionListener actionListner = new ActionListener() {
		public void actionPerformed(ActionEvent e) {

			for (int j = 0; j < getArrayButtons().length; j++) {
				for (int i = 0; i < getArrayButtons()[j].length; i++) {
					if (getArrayButtons()[j][i] == e.getSource()) {

						if ((gameNumber == 2)
								&& (playHand.getNumberOfCards() == 0)) {
							if (player[j].getCard(i).getSuit() == Suit.HEARTS
									.toString()
									&& player[j].hasSuitBesideHearts())
								// second game
								getPn10().setText("Can't play hearts");

							else {
								getArrayButtons()[j][i].setVisible(false);
								getPutImage()[j].setIcon(player[j].getCard(i)
										.getImage());
								getPnCardNumber()[j].setText(Integer
										.toString(player[j].getCard(i)
												.getNumber()));
								getPnCardName()[j].setText(player[j].getCard(i)
										.toString());
								getPnCardSuit()[j].setText(player[j].getCard(i)
										.getSuit());

								playHand.addCard(player[j].getCard(i), j);
								player[j].removeCard(i);

							}

						} else if ((gameNumber == 3)
								&& (playHand.getNumberOfCards() == 0)) {
							if ((playHand.getNumberOfCards() > 0)
									&& (playHand.getFirstSuit() != player[j]
											.getCard(i).getSuit())
									&& (player[j].hasSuit(playHand
											.getFirstSuit()))) {

								// third game
								getPn10().setText(
										"You must assist to "
												+ playHand.getFirstSuit());
							} else {
								getArrayButtons()[j][i].setVisible(false);
								getPutImage()[j].setIcon(player[j].getCard(i)
										.getImage());
								getPnCardNumber()[j].setText(Integer
										.toString(player[j].getCard(i)
												.getNumber()));
								getPnCardName()[j].setText(player[j].getCard(i)
										.toString());
								getPnCardSuit()[j].setText(player[j].getCard(i)
										.getSuit());

								playHand.addCard(player[j].getCard(i), j);
								player[j].removeCard(i);

							}

						} else {
							if ((playHand.getNumberOfCards() > 0)
									&& (playHand.getFirstSuit() != player[j]
											.getCard(i).getSuit())
									&& (player[j].hasSuit(playHand
											.getFirstSuit()))) {

								// first game
								getPn10().setText(
										"You must assist to "
												+ playHand.getFirstSuit());

							} else {
								getArrayButtons()[j][i].setVisible(false);

								getPutImage()[j].setIcon(player[j].getCard(i)
										.getImage());
								getPnCardNumber()[j].setText(Integer
										.toString(player[j].getCard(i)
												.getNumber()));
								getPnCardName()[j].setText(player[j].getCard(i)
										.toString());
								getPnCardSuit()[j].setText(player[j].getCard(i)
										.getSuit());

								playHand.addCard(player[j].getCard(i), j);
								player[j].removeCard(i);

							}
						}
					}
				}
			}
		}
	};

	/**
	 * New game.
	 * 
	 * @param number
	 *            the number
	 * @param text
	 */
	public void newGame(int number) {

		final Deck deck = new Deck();
		gameNumber = number;

		// shuffle the deck
		deck.shuffle();

		// Name of the game
		if (gameNumber == 1) {
			getOrder().setText("1st hand - No tricks");
		} else if (gameNumber == 2) {
			getOrder().setText("2nd hand - No Hearts");
		} else
			getOrder().setText("3rd hand - No Queens");

		// Create all Players
		for (int i = 0; i < player.length; i++) {
			player[i] = new Player();
		}

		// give cards to four players
		for (int i = 0; i < 13; i++) {
			for (int t = 0; t < player.length; t++) {
				player[t].receiveCard(deck.pickNextCard());
			}

		}

		// put cards on buttons
		for (int i = 0; i < getArrayButtons().length; i++) {
			for (int j = 0; j < getArrayButtons()[i].length; j++) {
				getArrayButtons()[i][j]
						.setText(player[i].getCard(j).toString());
				getArrayButtons()[i][j].setVisible(true);
			}
		}

		int hands = 0;
		int firstplayer = 0;
		int cardsEnabled = 0;

		// number of rounds of each game, change number to 13 if you want a
		// complete round. Now each game have three rounds for more easy
		// debugging
		while (hands < 3) {

			if (playHand.getNumberOfCards() < player.length) {

				try {
					Thread.sleep(30);
				} catch (InterruptedException e) {
					// TODO Auto-generated catch block
					e.printStackTrace();
				}
				if (cardsEnabled <= playHand.getNumberOfCards()) {
					setPlayer((firstplayer + playHand.getNumberOfCards()) % 4);
					cardsEnabled++;
				}
			}

			if (playHand.getNumberOfCards() == 4) {
				int winner = playHand.winner();
				int value = playHand.scoreFirstGame();

				// score
				if (winner == 0) {
					getText()[0].setText("Player 1 winner: "
							+ Integer.toString(value) + " negative");
				} else if (winner == 1) {
					getText()[1].setText("Player 2 winner: "
							+ Integer.toString(value) + " negative");
				} else if (winner == 2) {
					getText()[2].setText("Player 3 winner:  "
							+ Integer.toString(value) + " negative");
				} else if (winner == 3) {
					getText()[3].setText("Player 4 winner: "
							+ Integer.toString(value) + " negative");
				}

				try {
					Thread.sleep(2000);
				} catch (InterruptedException e) {
					// TODO Auto-generated catch block
					e.printStackTrace();
				}

				playHand.clearHand();
				for (int k = 0; k < getPutImage().length; k++) {
					getPutImage()[k].setIcon(null);
				}

				hands++;
				firstplayer = winner;
				cardsEnabled = 0;
			}
			number++;
		}
	}

	/**
	 * Sets the player.
	 * 
	 * @param playerEnabled
	 *            the new player
	 */
	public void setPlayer(int playerEnabled) {
		for (int i = 0; i < getArrayButtons().length; i++) {
			for (int j = 0; j < getArrayButtons()[i].length; j++) {
				if (playerEnabled == i) {
					getArrayButtons()[i][j].setEnabled(true);
					getArrayButtons()[i][j].setBackground(Color.white);

				} else {
					getArrayButtons()[i][j].setEnabled(false);
					Color greyHide = new Color(153, 153, 153);
					getArrayButtons()[i][j].setBackground(greyHide);
				}

			}
		}

	}

	public void setPnCardNumber(JTextField[] pnCardNumber) {
		this.pnCardNumber = pnCardNumber;
	}

	public JTextField[] getPnCardNumber() {
		return pnCardNumber;
	}

	public void setPnCardName(JTextField[] pnCardName) {
		this.pnCardName = pnCardName;
	}

	public JTextField[] getPnCardName() {
		return pnCardName;
	}

	public void setPnCardSuit(JTextField[] pnCardSuit) {
		this.pnCardSuit = pnCardSuit;
	}

	public JTextField[] getPnCardSuit() {
		return pnCardSuit;
	}

	public void setOrder(JTextField order) {
		this.order = order;
	}

	public JTextField getOrder() {
		return order;
	}

	public void setPn10(JTextField pn10) {
		this.pn10 = pn10;
	}

	public JTextField getPn10() {
		return pn10;
	}

	public void setPutImage(JLabel[] putImage) {
		this.putImage = putImage;
	}

	public JLabel[] getPutImage() {
		return putImage;
	}

	public void setLab(JLabel[] lab) {
		this.lab = lab;
	}

	public JLabel[] getLab() {
		return lab;
	}

	public void setArrayButtons(JButton[][] arrayButtons) {
		this.arrayButtons = arrayButtons;
	}

	public JButton[][] getArrayButtons() {
		return arrayButtons;
	}

	public void setText(JTextField[] text) {
		this.text = text;
	}

	public JTextField[] getText() {
		return text;
	}
}

if you need the GUI, i can post

thanks

[Junky]

Jeebus!

If you see that many levels if indentation then your code is screaming out for refactoring.

[Junky]

if you need the GUI, i can post

If all your GUI code is in another class then why in all that is holy do you have GUI stuff in KingData too.

DON'T DO THIS.

Keep it all in one or more classes that all they do is GUI stuff. Then you can access and modify gui components from another class as long as you have a reference to the GUI class(es). My code above may not be perfect but it certainly should help work this out.

Once again Google MVC pattern.

[felito]

i think the best thing i just cut the method new game into the gui class, yes i have another class. I can google MVC but for now i have to finalize this project so thanks, i really appreciate your help.

[Junky]

Nah. I can't be bothered.

[al_Marshy_1981]

i think the best thing i just cut the method new game into the gui class, yes i have another class. I can google MVC but for now i have to finalize this project so thanks, i really appreciate your help.

You need to cut your losses and redesign the whole thing. Keep classes specific to one thing i.e. A Car class will not have information about trains. Always when making member variables in a class ask yourself, does this class "have a..." e.g. does a Car have an umbrella? No, so don't put anything about umbrella's in here.

[felito]

thanks for the advice al :)