Ist dieser Code guter Code?

TomHH

TomHH

Mitglied
Schönen guten Abend :)

Ich weiß nicht, ob ich es so einfach darf, aber ich würde zu gerne wissen, ob mein Anfänger.Java ein gutes Java ist. Da ich gerade intensiv übe, wäre es sehr blöd, denke ich, wenn ich mir einen "schlechten" oder gar falschen Stil aneigne. Deshalb stelle ich mein Progrämmchen einfach mal hier zur Diskussion. Ich würde mich sehr freune das ein oder andere Feedback zu erhalten. Aber wenn es euch nervt ist es auch ok ;-) Dann schon mal herzlichen Dank! Tom

Code: 
/*
 * Created on 25.06.2004
 */
/**
 * @author Tom
 */

import java.math.*;

public class Aufgabe2 {
	
	
	final static int a = 60;
	final static int b = 24;
	final static int c = 30;
	final static int d = 365;
	private double it;
	private double sec;
	private double min;
	private double hour;
	private double day;
	private double month;
	private double year;
	
	private double getTime(){
		//	private: So gesicherter Zugriff?
		
		double given = IO.readDouble();
		return given;
	} 
	
	private double calculator() {
		//	private: So gesicherter Zugriff?
		
		sec = getTime();
		min = sec/a;
		hour = min/a;
		day = hour/b;
		month = day/c;
		year = month/d;
		it = year;
		return it;
	}
	
	void ausgabe(){
		//		...wäre schön, wenn man das "auslagern könnte und dann einfacher aufrufen
		//		anstatt jedes Mal den ganzen Code zu haben... => BigDecimal
		//		Aber es sind ja schon fertige Methoden und ich weiß nicht, ob man die noch 
		//		mal vereinfachend kapseln kann oder so was?
		BigDecimal nyear = new BigDecimal(year);
        BigDecimal aa = nyear.setScale(2, BigDecimal.ROUND_HALF_EVEN);
		System.out.println("Ihre Eingabe entspricht rund * " + aa + " * Jahren\n");
		BigDecimal nmonth = new BigDecimal(month);
        BigDecimal ab = nmonth.setScale(2, BigDecimal.ROUND_HALF_EVEN);
		System.out.println("Ihre Eingabe entspricht rund * " + ab + " * Monaten\n");
		BigDecimal nday = new BigDecimal(day);
        BigDecimal ac = nday.setScale(2, BigDecimal.ROUND_HALF_EVEN);
		System.out.println("Ihre Eingabe entspricht rund * " + ac + " * Tagen\n");
		BigDecimal nhour = new BigDecimal(hour);
        BigDecimal ad = nhour.setScale(2, BigDecimal.ROUND_HALF_EVEN);
		System.out.println("Ihre Eingabe entspricht rund * " + ad + " * Stunden\n");
		BigDecimal nmin = new BigDecimal(min);
        BigDecimal ae = nmin.setScale(2, BigDecimal.ROUND_HALF_EVEN);
		System.out.println("Ihre Eingabe entspricht rund * " + ae + " * Minuten\n");
		BigDecimal nsec = new BigDecimal(sec);
        BigDecimal af = nsec.setScale(2, BigDecimal.ROUND_HALF_EVEN);
		System.out.println("Ihre Eingabe entspricht rund * " + af + " * Sekunden");
	}
	
	public static void main(String[] args) {
		System.out.println("Geben Sie hier die Sekunden-Zahl ein, die umgerechnet werden soll: ");
		Aufgabe2 time = new Aufgabe2();
		time.calculator();
		time.ausgabe();
	}
}
 
Moin,
also folgende drei Verbesserungen habe ich vorzuschlagen:
1. Die Variablen a, b, c, d sind nicht als static zu deklarieren. Das ergibt einfach keinen Sinn. Ausserdem wäre eine sprechendere Variablenbezeichnung angebracht m.E. Schluss mit dem "Falschgeiz" bei der Anzahl der Variablenbezeichnung. ;)
Wenn eine Variable die Anzahl Fernseher beinhaltet, dann nenne sie doch bitte auch iAnzahlFernseher oder m_iAnzahlFernseher (m_ als Kennzeichnung für eine Membervariable=Instanzvariable, i für int). Dann weiss jeder Fremde, der den Code liest, sofort und jedes mal Bescheid, wenn diese Variable verwendet wird, von welchem Typ sie ist und was sie beinhaltet.

2. Funktioniert das Programm ohne Compiler-Schimpfe? Würde mich etwas wundern. Warum? Weil Du in einer static-Methode (main()) Zugriffe auf nicht-statische Methoden benutzt:
Code: 
time.calculator();
time.ausgabe();
Leg die beiden Methodenaufrufe lieber in den Konstruktor der Klasse. Also z.B.
Code: 
Aufgabe2()
{
 calculator();
 ausgabe();
}
Damit könnte
Code: 
time.calculator();
time.ausgabe();
aus der main-Methode entfernt werden.

Du könntest sogar, wenn Du absolut sicher bist, dass diese Klasse nicht mehr abgeleitet wird, die Methoden als final deklarieren.

3. Die Divisionen sind double / int. Vorschlag: Leg die Konstanten gleich als double an oder caste auf double, ist sauberer.

Apropos Divisionen:
Du kennst die böse Falle von Integer-Divisionen?
Tipp mal, ohne in einem Programm ausgeben zu lassen, was herauskommt bei

int iErgebnis = (int) 3 / (int) 2;

Was kommt heraus und warum?
 
Original geschrieben von Snape
2. Funktioniert das Programm ohne Compiler-Schimpfe? Würde mich etwas wundern. Warum? Weil Du in einer static-Methode (main()) Zugriffe auf nicht-statische Methoden benutzt:
Zum Vergrößern anklicken....

<TOM> Ja, funkt ohne murren ;-) Liegt vielleicht an Eclipse!? <TOM />
Code: 
time.calculator();
time.ausgabe();
Leg die beiden Methodenaufrufe lieber in den Konstruktor der Klasse. Also z.B.
Code: 
Aufgabe2()
{
 calculator();
 ausgabe();
}
Damit könnte
Code: 
time.calculator();
time.ausgabe();
aus der main-Methode entfernt werden.
Zum Vergrößern anklicken....
<TOM> OK. Nur wie rufe ich dann die Methoden auf? Mit Aufgabe2.ausgabe();? Ich dachte bislang, man müsse immer ein Objekt erzeugen und dieses dann aufrufen bzw. seine Methoden?<TOM />
Du könntest sogar, wenn Du absolut sicher bist, dass diese Klasse nicht mehr abgeleitet wird, die Methoden als final deklarieren.
Zum Vergrößern anklicken....
<TOM> Und welchen Vorteil hat das? Schutz?<TOM />
3. Die Divisionen sind double / int. Vorschlag: Leg die Konstanten gleich als double an oder caste auf double, ist sauberer.
Zum Vergrößern anklicken....
<TOM> Na riesig. Rate mal, wie ich es ursprünglich hatte? ;-)<TOM />
Apropos Divisionen:
Du kennst die böse Falle von Integer-Divisionen?
Zum Vergrößern anklicken....
<TOM> JEIN! Aber nicht mehr genau, ob der Teiler oder die zu teilende Zahl als double deklariert sein muss...<TOM />
Tipp mal, ohne in einem Programm ausgeben zu lassen, was herauskommt bei

int iErgebnis = (int) 3 / (int) 2;

Was kommt heraus und warum?
Zum Vergrößern anklicken....
<TOM> iErgebnis = 1, da die zu teilende Zahl (?) nicht double ist bzw die Variable iErgebnis... Da war auf jeden Fall was ;-)<TOM />

<TOM>Thx Snape!<TOM />
&nbsp;
 
Original geschrieben von Snape

Leg die beiden Methodenaufrufe lieber in den Konstruktor der Klasse. Also z.B.
Code: 
Aufgabe2()
{
 calculator();
 ausgabe();
}
Zum Vergrößern anklicken....
<TOM> Muss der Konstruktor noch Deklarationen erhalten? Also z.B. private oder so was? <TOM />
Damit könnte
Code: 
time.calculator();
time.ausgabe();
aus der main-Methode entfernt werden.
Zum Vergrößern anklicken....
<TOM> Snape, ich hab's:
Code: 
Aufgabe2()
	{
	 calculator();
	 ausgabe();
	}
	
	public static void main(String[] args) {
		System.out.println("Geben Sie hier die Sekunden-Zahl ein, die umgerechnet werden soll: ");
		Aufgabe2 time = new Aufgabe2();

Wenn ich das richtig sehe, dann wird ein Objekt erzeugt und sämtliche Klassen, Methoden und Eigenschaften an dieses übergeben. Durch den Konstruktor Aufgabe2() werden dann die Methoden aufgerufen und abgearbeitet!? Stimmt das so ungefähr? Cooles Ding das! Freu mich gerade wie Oscar :)<TOM />
&nbsp;
 
Verbesserter Code ist nun...

Code: 
/*
 * Created on 25.06.2004
 */
/**
 * @author Tom
 */

import java.math.*;

public class Aufgabe2 {
	
	final double dTeiler60 = 60;
	final double dTeiler24 = 24;
	final double dTeiler30 = 30;
	final double dTeiler365 = 365;
	private double it;
	private double sec;
	private double min;
	private double hour;
	private double day;
	private double month;
	private double year;
	
	private double getTime(){
		
		double given = IO.readDouble();
		return given;
	} 
	
	private double calculator() {
		
		sec = getTime();
		min = sec/dTeiler60;
		hour = min/dTeiler60;
		day = hour/dTeiler24;
		month = day/dTeiler30;
		year = month/dTeiler365;
		it = year;
		return it;
	}
	
	void ausgabe(){
		//		...wäre schön, wenn man das "auslagern könnte und dann einfacher aufrufen
		//		anstatt jedes Mal den ganzen Code zu haben... => BigDecimal
		//		Aber es sind ja schon fertige Methoden und ich weiß nicht, ob man die noch 
		//		mal vereinfachend kapseln kann oder so was?
		BigDecimal nyear = new BigDecimal(year);
        BigDecimal aa = nyear.setScale(2, BigDecimal.ROUND_HALF_EVEN);
		System.out.println("Ihre Eingabe entspricht rund | " + aa + " | Jahren\n");
		BigDecimal nmonth = new BigDecimal(month);
        BigDecimal ab = nmonth.setScale(2, BigDecimal.ROUND_HALF_EVEN);
		System.out.println("Ihre Eingabe entspricht rund | " + ab + " | Monaten\n");
		BigDecimal nday = new BigDecimal(day);
        BigDecimal ac = nday.setScale(2, BigDecimal.ROUND_HALF_EVEN);
		System.out.println("Ihre Eingabe entspricht rund | " + ac + " | Tagen\n");
		BigDecimal nhour = new BigDecimal(hour);
        BigDecimal ad = nhour.setScale(2, BigDecimal.ROUND_HALF_EVEN);
		System.out.println("Ihre Eingabe entspricht rund | " + ad + " | Stunden\n");
		BigDecimal nmin = new BigDecimal(min);
        BigDecimal ae = nmin.setScale(2, BigDecimal.ROUND_HALF_EVEN);
		System.out.println("Ihre Eingabe entspricht rund | " + ae + " | Minuten\n");
		BigDecimal nsec = new BigDecimal(sec);
        BigDecimal af = nsec.setScale(2, BigDecimal.ROUND_HALF_EVEN);
		System.out.println("Ihre Eingabe entspricht rund | " + af + " | Sekunden");
	}
	
	Aufgabe2()
	{
	 calculator();
	 ausgabe();
	}
	
	public static void main(String[] args) {
		System.out.println("Geben Sie hier die Sekunden-Zahl ein, die umgerechnet werden soll: ");
		Aufgabe2 time = new Aufgabe2();
	}
}
 
> Muss der Konstruktor noch Deklarationen erhalten? Also z.B. private oder so was?

Nö. Üblicherweise hat er keine, es sei denn, er wird aus einer anderen Klasse aus einem anderen package aufgerufen, dann muss ein public davor.

> Snape, ich hab's:
Code: 
Aufgabe2()
	{
	 calculator();
	 ausgabe();
	}
	
	public static void main(String[] args) {
		System.out.println("Geben Sie hier die Sekunden-Zahl ein, die umgerechnet werden soll: ");
		Aufgabe2 time = new Aufgabe2();

Hm, das ähnelt irgendwie dem Code in meiner ersten Antwort. ;)

>Wenn ich das richtig sehe, dann wird ein Objekt erzeugt und sämtliche Klassen, Methoden und Eigenschaften an dieses übergeben.

Nö. Wo und wie sollte das denn geschehen?

>Durch den Konstruktor Aufgabe2() werden dann die Methoden aufgerufen und abgearbeitet!? Stimmt das so ungefähr? Cooles Ding das! Freu mich gerade wie Oscar :)

DAS stimmt 100%.
Durch Aufgabe2 time = new Aufgabe2(); wird der Konstruktor von Aufgabe2 aufgerufen und ausgeführt. D.h. alles, was im Konstruktor passiert, wird ausgeführt. In diesem Fall eben calculator(); und ausgabe();
 
a) Deutsche Begriffe haben nichts im Quellcode zu suchen. class Aufgabe2 oder auch ausgabe() haben im Code nichts zu suchen.

Mag sein das dein Programm selbst deutsche Kommentare enthält, aber der Quellcode selbst sollte englisch sein. Versuche dich daran zu gewöhnen ;o)

(Ja, ich hab mich schon einmal geärgert weil jemand ein nützliches Programm in spanisch verfasst hat. Normalerweise kann ich mir ja aufgrund der Sourcen selbst die Vorgehensweise abgucken, aber in dem Falle ist dann Hopfen und Malz verloren. Liest sich fast wie obfuscated Code).

b) double it;
Wenn du in it immer redundant den Wert von year hast, dann scheint mir it selbst überflüssig zu sein.

c)

Wieso verwendest du dann am Ende BigDecimal?

Würde dir nicht ein
System.out.println("Ihre Eingabe entspricht rund | " + Math.round(month) + " | Monaten\n");
reichen?

cybi
 
<TOM> Und welchen Vorteil hat das? Schutz?<TOM />

Nö, ist eigentlich nur für den Compiler interessant. Es ist aber nie ganz klar, ob und wenn ja welcher Performance-Vorteil für das laufende Programm herausspringt.

<TOM> iErgebnis = 1, da die zu teilende Zahl (?) nicht double ist bzw die Variable iErgebnis... Da war auf jeden Fall was ;-)<TOM />

Richtig geraten. ;)
 
Hallo.

Ich habe mal gelernt/gelesen, dass final variablen immer GROSS geschrieben werden.

In meinem Praxissemsester war ausserdem Bulgarische Notaion verboten,also
iAnzahlFernseher (i für int)

mfg
phil
 
Die bulgarische Notation ist eigentlich in Wirklichkeit eine ungarische ;o)

Und ja diese Notation ergibt in Java keinen Sinn. Es ist besser auf diese Notation zu verzichten, denn sie hat mehr Probleme als Nutzeffekte.

Beispiel:
char iBlub;
Was ist es nun? Integer oder Char? ;o)

Das es ein char ist wußte ich eigentlich auch obwohl da gar kein Präfix steht ..
Spätestens mit den Generics hat die ungarische Notation keine Daseinsberechtigung.

Im übrigen ist es wirklich so, das Konstanten (finals sind Konstanten) GROSS geschrieben werden.

cybi
 
Um antworten zu können musst du eingeloggt sein.

Neue Beiträge

Zurück