Codeverbesserung, FH-Hausaufgabe | Bruchrechner

[h4nk]

Hallo Leute, folgende Aufgabe:

Schreiben Sie ein Programm Bruchautomat, welches die Klasse Bruchautomat enthält und das Package Bruch. Ausserdem schreiben Sie eine Klasse Bruch als Datenklasse und eine Kasse I_O als Ein_Ausgabe-Klasse.

Die Datenklasse wird von der Klasse Bruchautomat benutzt, ebenso die I_O Klasse. Die Klasse Bruchautomat enthält die „main-methode“. In der main-methode sind zuerst die Eingabe Methoden der Klasse I_O aufzurufen , um die beiden Brüche einzugeben. (siehe unten). Ausserdem ist der Operator einzugeben, der zeigt, welche der Bruch-Funktionen aufgerufen werden soll.
Danach ist über die Auswahl des Operators (+ - * / ) die entsprechende Mitgliedsfunktionen der Klasse Bruch aufzurufen.Das Ergebnis der mathematischen Operation ist gekürzt auszugeben. Die Funktion „Kürzen“ hat die Form: private Bruch kuerzen(Bruch b) und ist in der Klasse Bruch zu plazieren.
Das Ganze soll wiederholbar sein, bis ein Abbruchkriterium das Programm beendet. Fehler sind abzufangen.

Tests:
a) Gutfall ohne Kürzen: +Habe ich wie folgt gelöst: - * /
b) Gutfall mit Kürzen: + - * /
c) Teilen durch '0' (Kehrwert berücksichtigen)
d) Wer's kann: Eingabe von Sonderzeichen abfangen (optional)

Habe ich wie folgt gelöst:
Bruchklasse

public class Bruch
{
    private int zaehler;
    private int nenner;
   
    public Bruch addieren(Bruch br2)
    {
        Bruch erg = new Bruch();
        int z, n;
       
        z = (zaehler * br2.getNenner()) + (br2.getZaehler() * nenner);
        n = nenner * br2.getNenner();
       
        erg.setZaehler(z);
        erg.setNenner(n);
       
        return erg;
    }
   
   
    public Bruch subtrahieren(Bruch br2)
    {
        Bruch erg = new Bruch();
        int z, n;
       
        z = (this.getZaehler() * br2.getNenner()) - (br2.getZaehler() * this.getNenner());
        n = this.getNenner() * br2.getNenner();
       
        erg.setZaehler(z);
        erg.setNenner(n);
       
        return erg;
    }
   
    public Bruch multiplizieren(Bruch br2)
    {
        Bruch erg = new Bruch();
        int z, n;
       
        z = this.getZaehler() * br2.getZaehler();
        n = this.getNenner() * br2.getNenner();
       
        erg.setZaehler(z);
        erg.setNenner(n);
       
        return erg;
    }
   
    public Bruch dividieren(Bruch br2)
    {
        Bruch erg = new Bruch();
        int z, n;
       
        z = this.getZaehler() * br2.getNenner();
        n = this.getNenner() * br2.getZaehler();
       
        erg.setZaehler(z);
        erg.setNenner(n);
       
        return erg;
    }
   
    public Bruch kuerzen()
    {
        int zahl1 = this.getZaehler();
        int zahl2 = this.getNenner();
       
        while (zahl2 != 0)
        {
            if (zahl1 > zahl2)
            {
                zahl1 = zahl1 - zahl2;
            }
            else
            {
                zahl2 = zahl2 - zahl1;
            }
        }
       
        int ggT = zahl1;
       
        this.setZaehler(this.getZaehler() / ggT);
        this.setNenner(this.getNenner() / ggT);

        return this;
    }
   
   
    public int getZaehler()
    {
        return zaehler;
    }
   
    public void setZaehler(int zaehler)
    {
        this.zaehler = zaehler;
    }
   
    public int getNenner()
    {
        return nenner;
    }
   
    public void setNenner(int nenner)
    {
        this.nenner = nenner;
    }
}

Eingabe/Ausgabe

package view;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

import model.Bruch;

public class I_O
{
    BufferedReader buffer = new BufferedReader(new InputStreamReader(System.in));

    public char operator() throws IOException
    {
        System.out.println("");
        System.out.println("Bitte Operator eingeben: ");
       
        String oper = buffer.readLine();
        char operator = oper.charAt(0);
        return operator;
    }
   
   
    public Bruch eingabe(int count) 
    {
        int z = 0;
        int n = 0;
       
        boolean nichtGueltig = false;
        Bruch br = new Bruch();
       
        do
        {
            try
            {
                System.out.println("Bitte " + count + ". Zähler eingeben.");
                z = Integer.parseInt(buffer.readLine());
                nichtGueltig = false;
            }
            catch (Exception e)
            {
                System.out.println("");
                System.out.println("Fehler bei der " + count + ". Zählereingabe. \n");
                nichtGueltig = true;
            }
        } while (nichtGueltig);
       
        br.setZaehler(z);
       
        do
        {
            try
            {
                System.out.println("Bitte " + count + ". Nenner eingeben.");
                n = Integer.parseInt(buffer.readLine());
                if (n == 0)
                {
                    System.out.println(" ");
                    System.out.println("Null ist keine gültige Eingabe. \n");
                    nichtGueltig = true;
                }
                else
                {
                    nichtGueltig = false;
                }
            }
           
            catch (Exception e)
            {
                System.out.println("");
                System.out.println("Fehler bei der " + count + ". Nennereingabe. \n");
                nichtGueltig = true;
            }
        } while (nichtGueltig);
       
        br.setNenner(n);
       
        return br;
    }
   
    public Bruch eingabe(int count, char operator)
    {
        int z = 0;
        int n = 0;
       
        boolean nichtGueltig = false;
        Bruch br = new Bruch();
       
        do
        {
            try
            {
                System.out.println("Bitte " + count + ". Zähler eingeben.");
                z = Integer.parseInt(buffer.readLine());
                if (operator == '/' && z == 0)
                {
                    System.out.println(" ");
                    System.out.println("Null ist keine gültige Eingabe. \n");
                    nichtGueltig = true;
                }
                else
                {
                    nichtGueltig = false;
                }
            }
            catch (Exception e)
            {
                System.out.println("");
                System.out.println("Fehler bei der " + count + ". Zählereingabe. \n");
                nichtGueltig = true;
            }
        } while (nichtGueltig);
       
        br.setZaehler(z);
       
        do
        {
            try
            {
                System.out.println("Bitte " + count + ". Nenner eingeben.");
                n = Integer.parseInt(buffer.readLine());
                if (n == 0)
                {
                    System.out.println(" ");
                    System.out.println("Null ist keine gültige Eingabe. \n");
                    nichtGueltig = true;
                }
                else
                {
                    nichtGueltig = false;
                }
            }
           
            catch (Exception e)
            {
                System.out.println("");
                System.out.println("Fehler bei der " + count + ". Nennereingabe. \n");
                nichtGueltig = true;
            }
        } while (nichtGueltig);
       
        br.setNenner(n);
       
        return br;
    }
   
   
    public void ausgabe(Bruch br1, Bruch br2, char operator, Bruch erg)
    {
        System.out.println("");
        System.out.println(
                br1.getZaehler() + " / " + br1.getNenner() + " " + operator + " " +
                br2.getZaehler() + " / " + br2.getNenner() + " = " +
                erg.getZaehler() + " / " + erg.getNenner());
    }
   
   
    public boolean nochmal() throws IOException
    {
        boolean eingabe = true;
        System.out.println(" ");
        System.out.println("Nochmal? (J/N)");
       
        while (eingabe)
        {
            String jn = buffer.readLine();
           
            if (jn.equals("J") || jn.equals("j"))
            {
                return true;
            }
            else if ((jn.equals("N") || jn.equals("n")))
            {
                return false;
            }
            else
            {
                System.out.println("Falsche Eingabe, bitte erneut eingeben");
            }
        }
        return false;
    }
}

Bruchautomat

package controller;

import java.io.IOException;

import model.Bruch;
import view.I_O;

public class Bruchautomat {

    public static void main(String[] args) throws IOException
    {
        boolean fortsetzen = true;
       
        do
        {
            Bruchautomat ba = new Bruchautomat();
            I_O io = new I_O();
           
            Bruch br1 = new Bruch();
            br1 = io.eingabe(1);
           
            char operator = io.operator();
           
            System.out.println("");
            Bruch br2 = new Bruch();
            br2 = io.eingabe(2, operator);
           
           
           
            Bruch erg = new Bruch();
           
            erg = ba.berechne(br1, br2, operator, io);
            erg.kuerzen();
           
            io.ausgabe(br1, br2, operator, erg);
       
            fortsetzen = io.nochmal();
        }
        while (fortsetzen);
    }
   
   
    public Bruch berechne(Bruch br1, Bruch br2, char operator, I_O io) throws IOException
    {
        Bruch result = new Bruch();
       
        switch (operator)
        {
        case '+' :    result = br1.addieren(br2);
                    break;
                   
        case '-' :    result = br1.subtrahieren(br2);
                    break;
                   
        case '*' :    result = br1.multiplizieren(br2);
                    break;
                   
        case '/' :    result = br1.dividieren(br2);
                    break;
        }
        return result;
    }
}

Mich interessiert was ich besser machen kann, da ich noch recht neu in der Programmierung bin Ich denke, nur so komme ich voran.

Also Feuer frei! (Falls überhaupt jemand Lust hat das komplett zu lesen )

Danke schonmal!

 

[Kababär]

Für meinen Geschmack zu viele while-Schleifen... hab mir den Code nicht ganz genau angeguckt, aber die while-Schleife in der nochmal-Methode ist überflüssig?
Generell: deine while-Bedingung ist eine while-Bedingung und das finde ich nicht so prickelnd.
Brauchst du unbedingt alle while-Schleifen?

 

[JStein52]

die while-Schleife in der nochmal-Methode ist überflüssig?

Nein, ist sie nicht, da wartet er bis der Benutzer entweder j/J oder n/N eingibt. er könnte aber genausogut "while (true)" schreiben. Und auch bei den anderen Schleifen wartet er so lange bis der Benutzer etwas sinnvolles eingibt also sind die auch gerechtfertigt. Man könnte nur überlegen ob man diese Eingaben nicht in eine einzige Methode verlagern kann die das dann handled.

 

[Kababär]

Na gut klar kann mans so machen. Ich persönlich würde lieber eine komplette Eingabe auf einmal verarbeiten (wenn der Benutzer enter drückt). Denn so könnte ich das Programm von der Konsole aufufen und ihm einfach die Eingabe als Parameter mitgeben.

Edit: Hab Mist erzählt

 

[Joose]

Dein Code ist gut formatiert und verständlich.
"Besser machen" kann man bei Code vieles, man kann die Perfomance verbessern, die Wartbarkeit, die Lesbarkeit, ...
Wichtig ist dass du dich selber auskennst und den Code verstehst.
Ich habe mal ein paar Anmerkungen zusammengeschrieben.

Zu Bruchautomat:
In der main-Methode erzeugst du in der do/while-Schleife 3 Bruch Objekte unnötig. Die Methoden eingabe und berechne geben dir direkt schon ein Bruch Objekt zurück.
Sprich deine erzeugten Objekte in der main-Methode werden sowieso sofort wieder weggeworfen.
Außerdem erzeugst du jedesmal ein neues Objekt von I_O und Bruchautomat, muss nicht sein.

   public static void main(String[] args) throws IOException
   {
     boolean fortsetzen = true;
     Bruchautomat ba = new Bruchautomat();
     I_O io = new I_O();

     do
     {
       Bruch br1 = io.eingabe(1);
       char operator = io.operator();
       Bruch br2 = io.eingabe(2, operator);

       Bruch erg = ba.berechne(br1, br2, operator, io);
       
       io.ausgabe(br1, br2, operator, erg);       
       fortsetzen = io.nochmal();
     }
     while (fortsetzen);
   }

In der Methode berechne erzeugst du auch ein unnötiges Bruch Objekt.

Zu Bruch:
Ich würde der Klasse Bruch einen Konstruktor spendieren der als Parameter zaehler und nenner entgegen nimmt.
Dadurch muss man nicht immer erst den Defaultkonstruktor aufrufen und dann die setter Methoden.
Auch habe ich der Angabe entnommen das die Methode kuerzen private sein und immer aufgerufen werden soll.

   public Bruch(int zaehler, int nenner)
   {
     this.zaehler = zaehler;
     this.nenner = nenner;
   }

   public Bruch addieren(Bruch br2)
   {
     int z = (zaehler * br2.getNenner()) + (br2.getZaehler() * nenner);
     int n = nenner * br2.getNenner();

     return new Bruch(z, n).kuerzen();
   }

In dieser Klasse ist klar was "z" bzw. "n" ist, aber man sollte generell sprechende Namen verwenden. Also vielleicht etwas wie "neuerZaehler", "neuerNenner".

Zu I_O:
Vermeide "_" in Klassennamen, Attribute oder Methodennamen!
Die beiden Methoden eingabe schauen sich sehr ähnlich. Vor allem die darin enthaltenen (4) do/while-Schleifen sind nahezu identisch.
Überlege dir ob du diesen Teil vielleicht in eine eigene Methode auslagern kannst. Dadurch hast du nur eine Stelle zum Debuggen und anpassen und der Code wird auch kürzer.
In der Methode nochmal deklarierst du zwar eine Abbruchbedingung für deine Schleife, diese wird aber niemals gesetzt
Entweder du arbeitest mit dieser Abbruchbedingung oder du sparst sie dir ein und machst eine while(true) daraus (wovon ich abrate).
Auch kannst du die Methode toLowerCase verwenden um deine Eingabe auf Kleinbuchstaben umzuwandeln, dadurch ersparst du dir die Abfrage auf "J" bzw. "N".

   public boolean nochmal() throws IOException
   {
     boolean falscheEingabe = true;
     boolean ergebnis = false;
     System.out.println("");
     System.out.println("Nochmal? (J/N)");

     while (falscheEingabe)
     {
       String jaNein = buffer.readLine().toLowerCase();
       if (jaNein.equals("j"))
       {
         falscheEingabe = false;
         ergebnis = true; // ergebnis muss nur auf true gesetzt werden da der initialwert sowieso false ist.
       }
       else if (jaNein.equals("n")))
       {
         falscheEingabe = false;
       }
       else
       {
         System.out.println("Falsche Eingabe, bitte erneut eingeben");
       }
     }
     return ergebnis;
   }

 

[h4nk]

So, nachdem ich endlich mal Zeit habe, möchte ich euch danken für die hilfreichen Antworten. Besonders dir, Joose.

Ich habe alle Änderungen aufgenommen und umgesetzt. Jetzt wird die Dokumentation fertiggestellt und morgen abgegeben! Schönes Wochenende. Der Thread kann zu.