Schlechte Programmierung

[Semptic]

Hallo,

ich habe ein schlechtes Gewissen dabei wie ich mein Programm schreibe, denn ich übergebe einigen meiner Klassen ihren Erzeuger und greife dann in der Klasse auf Funktionen des Erzeugers zu. Dies deshalb um nicht mehrere andere Objekte übergeben zu müssen.

Sieht dann so aus:
MainWindow:

public class MainWindow extends JFrame implements ActionListener, ListSelectionListener{
	
	private JTable iniTable;	
	PersonOverview personOverview;
	
	private ArrayList<Person> personen = new ArrayList<Person>();
	//Constructors
	public MainWindow() {
		super("Initiative Manger");
    	
    	GridBagLayout gridL = new GridBagLayout();
    	setLayout(gridL);
    	GridBagConstraints gridC = new GridBagConstraints();
    	
    	//Table
		iniTable = new JTable(new InITable());
			//Table config
			iniTable.setSelectionMode(ListSelectionModel.SINGLE_SELECTION);
			iniTable.setFillsViewportHeight(true);	
			iniTable.setModel(new InITable());
			iniTable.setMinimumSize(new Dimension(150, 150));
			iniTable.setSize(150, 150);
			
			ListSelectionModel lsm = iniTable.getSelectionModel();
	    	lsm.addListSelectionListener(this);
	    	iniTable.setSelectionModel(lsm);
    		//TablePane
			JScrollPane tablePane = new JScrollPane(iniTable);
    			//Layout
    			gridC.gridx = 0;
    			gridC.gridy = 0;
    			gridC.gridheight = 2;
    			gridC.gridwidth = 1;
    			gridC.anchor = GridBagConstraints.NORTH;
    			//gridC.fill = GridBagConstraints.BOTH;
    			gridL.setConstraints(tablePane, gridC);
		add(tablePane);
		
		personOverview = new PersonOverview(this);
			gridC.gridx = 1;
			gridC.gridy = 0;
			gridC.gridheight = 2;
			gridC.gridwidth = 1;
			gridC.fill = GridBagConstraints.NONE;
			gridL.setConstraints(personOverview, gridC);
		add(personOverview);  	
    }
	//Worker
	public void repaintTable() {
		int selectedRow = iniTable.getSelectedRow();
		Person selectedPerson;
		
		if (!(selectedRow < 0)) {
			selectedPerson = personen.get(selectedRow);
		} else {
			selectedPerson = new Person("Error");
		}
		
		if (!(selectedRow < 0)) {
			iniTable.changeSelection(personen.indexOf(selectedPerson), 0, false, false);
		}
		
		iniTable.repaint();
	}	
	public void deletePerson(Person person) {
		int i = personen.indexOf(person);
		iniTable.clearSelection();
		personen.remove(i);
		iniTable.changeSelection(i, 0, false, false);
		repaintTable();
	}
	//Listener
	public void valueChanged(ListSelectionEvent e) {
		int i = iniTable.getSelectedRow();
		if (!e.getValueIsAdjusting() && i >= 0) {
			personOverview.setPerson(personen.get(i));
		}
		
	}
}

PersonOverView

public class PersonOverview extends JPanel implements ActionListener{

	private Person person;

	//Constructors
	public PersonOverview(MainWindow owner) {
		setLayout(gridL);
		this.owner = owner;
		//blubb GUI blubb
	}
	
	public void setPerson(Person person) {
		//blub
	}

	@Override
	public void actionPerformed(ActionEvent arg0) {
		Object sender = arg0.getSource();
		 if (sender == delete) {
			owner.deletePerson(person);			
		}
	}
}

PersonOverview erzeugt auch noch eine Instanz einer Klasse welche die Referenz auf MainWindow übergeben bekommt, und auf die Methode

repaintTable()

zugreift.

Sowas ist wirklich unsauber und ich sollte besser die benötigten Referenzen erzeugen, oder täuscht mich da mein gefühl?

€dit: Beispiel verändert

 

[Mujahiddin]

Also in deinem Beispiel hätte ich das umgekehrt gemacht. Dass Mainwindow ein Objekt der Klasse MyClass ist... dann könnte MyClass direkt MainWindow.doOtherThings() aufrufen und MainWindow bräuchte keine Referenz auf MyClass.

Allgemein finde ich persönlich auch, dass sich zwei Klassen nicht gegenseitig kennen sollten. Das hat sich aber glaub ich nur bei mir eingebürgert, immer häufiger sehe ich in letzter Zeit sowas.

Du könntest auch mit dem Observer arbeiten... Ansonsten müsstest du auf andere Meinungen warten

 

[Semptic]

Hab mein Beispiel wohl schlecht gewählt, werd des noch mal überarbeiten, dass es aussagekräftiger ist.

 

[Michael...]

In Deinem Bsp ist die Klasse MyClass leider überflüssig. Hast Du nicht ein konkreteres Bsp?

 

[Semptic]

Hab mal des Beispiel geändert, hoffe das es jetzt erkenntlich ist

 

[Michael...]

In dem Fall würde ich die Implementierung des ActionListeners aus PersonOverview rausnehmen und in MainWindow stecken, da ja MainWindow die Klasse ist die als Listener auf Actions in PersonOverview reagieren muss und eine solche Implementierung in PersonOverview am falschen Platz ist.