Android Ist das ein akzeptabler Code?

[rahu2000]

Hallo zusammen,

ich bin erst seit kurzem beim Java-Programmieren und habe deshalb eine Frage zu meinem Code-Stil:

public void onCreate(Bundle savedInstanceState) 
	{	
		super.onCreate(savedInstanceState);
		setContentView(R.layout.main);
	
		myButton = (Button)findViewById(R.id.Button01);
		myButton2 = (Button)findViewById(R.id.Button02);
		myLabel = (TextView)findViewById(R.id.TextView01);
		
		setOnClickList(myButton);
		setOnClickList(myButton2);
	}
	
	public void setOnClickList(final Button but)
	{	
		but.setOnClickListener(new OnClickListener() 
		{
	        public void onClick(View v) 
	        {
	        	if (but == myButton) 
	        	{
	        		but.setText("Hellooo1!");
	        	}
	        	if (but == myButton2) 
	        	{
	        		but.setText("Hellooo2!");
	        	}
	        }
	      });
	}

Mir geht es vorallem um die "setOnClickList" - Methode, da ich nich weiß ob das vom Code her annehmbar oder furchtbar schrecklich ist. Denn in online-Codeschnipseln findet man das immer so:

myButton.setOnClickListener(new View.OnClickListener() {
public void onClick(View v) {
myLabel.setText("Blabla");

}
});

und ich persönlich finde das unübersichtlich und hab das deshalb in eine extra-Methode gepackt (kommt vielleicht daher, dass ich vorher zu viel Visual Basic programmiert habe)

Findet ihr -vorallem die erfahrenen Programmierer - , dass man das so lassen kann oder sollte ich das anders schreiben?

Vielen Dank für eure Meinung im Voraus und

mfG

rahu2000

 

[mjdv]

Generell solltest du den Objekten, also den Buttons sinnvolle Namen geben wie zB loginButton oder cancelButton.

Die Methode ist an sich ok, aber eigentlich macht sie nicht das was du glaubst was sie tut denke ich. Du erstellst zwei mal das OnClick Objekt, dh. eigentlich völlig sinnfrei.

Sowas zB wäre besser:

private OnClickListener myListener  = new OnClickListener() {
 
        @Override
        public void onClick(View v) 
            {
               int viewId = v.getId();
                if (viewId == R.id.button1) 
                {
                    but.setText("Hellooo1!");
                }
                if (viewId == R.id.button2) 
                {
                    but.setText("Hellooo2!");
                }
            }
}

...
but1.setOnClickListener(myListener);
but2.setOnClickListener(myListener);
...

Damit erstellst du den listener nur einmal.

Vll schaußt du dir das mit den Listenern bei Swing nochmal an. Da gibt es so viele verschiedene Möglichkeiten

 

[Marco13]

Bei Android gelten wohl andere Regeln und Konventionen, als bei "normalem" Java. (Ja, es ist eigentlich das gleiche, und es funktioniert wohl beides auf die eine oder andere Weise, aber die "best practices" können doch unterschiedlich sein). Deswegen der Disclaimer vorweg: Das bezieht sich jetzt NICHT speziell auf Android!

Üblicherweise (bei "normalem" Java/Swing) ist IMHO und aus subjektiver Erfahrung heraus weniger das Anhängen des Listeners das kritische, sondern eher das, was der Listener macht.

Falls dort mehr gemacht wird als sowas wie [c]but.setText("Hellooo1!");[/c], dann sollte man das möglichst schnell in eine private Methode auslagern. Das gilt für beide Fälle: Sowohl für deine Lösung, als auch für das, was du als unübersichtlich bezeichnet hast.


Darüber hinaus... Erstmal abwarten, ob mich ein Android-Experte was dazu sagt Hier kommt nur noch subjektiv-unfundierter Kram:

Ich finde sowas wie

myButton.setOnClickListener(new View.OnClickListener() {
    public void onClick(View v) {
        doSomething();
    }
});

gar nicht so unübersichtlich: Man sieht direkt, dass ein Klick auf "myButton" einen Aufruf von "doSomething" bewirkt. Dieses Herstellen der Verbindung zwischen Button und Funktion steht so im Code sehr kompakt an einer Stelle. Bei der von dir vorgeschlagenen Lösung muss der Anonyme OnClickListener immer wissen, auf welche Buttons er hört, und welche er prüfen muss - die Wartung dieses Listeners könnte bei "vielen" Buttons irgendwann unübersichtlich werden.
Mit Java8 wird sich das ganze dann ja voraussichtlich in sowas wie

myButton.setOnClickListener( #doSomething(); );

verwandeln.... :reflect:

 

[rahu2000]

Vielen Dank für die Antworten.
Ich habe eine Frage zu mjdv's Vorschlag:

Der Compiler gibt nur Ruhe wenn ich die

private OnClickListener myListener..,

-Methode mit einem ";" abschließe. - Wieso?

mfG

rahu2000

PS: @ Marco13: Du hast recht wenn ichs mir so überlege wird es doch unübersichtlicher mit steigender Anzahl an Buttons und längerem Quellcode...

 

[mjdv]

Stimmt hab ich vergessen sry.

Naja es ist eigentlich ein ganz normales instanziieren eines Objekts. Wie als wenn du Object o = new Object(); machst, da muss ja auch das ';' hin. Da es sich hier aber eben um ein Interface handelt musst du eben ncoh die Methoden entsprechend implementieren.

 

[rahu2000]

Ahhh jetzt hab ichs verstanden. Onclicklistener ist also ein interface;-) Vielen Dank!

mfG
rahu2000

 

[fastjack]

Die vielen if's würde ich erstmal durch else if(...) ersetzen, da sonst alles durchgeprüft wird und es kann ja nur ein Button geklickt sein.

edit: Die Java-Code-Conventions darf man auch in Android-Programmen nutzen

 

[schlingel]

Wie Eingangs erwähnt, gibt es Unterschiede in der Entwicklung für Android u. Java SE/EE. In deinem Beispiel finde ich den Umweg unnötig.

Entweder du erzeugst für beide Buttons einen individuellen Handler oder du implementierst in der Activity-Klasse den Handler. Damit würdest du dir Objekte sparen. Geht allerdings ein wenig auf Kosten der Lesbarkeit.