Datentypimplementierung für rationale Zahlen

Question

Dies ist Übung 3.2.7. aus dem Buch Informatik Ein interdisziplinärer Ansatz von Sedgewick & Wayne:

Implementieren Sie einen Datentyp für rationale Zahlen, der Addition, Subtraktion, Multiplikation und Division unterstützt.

Hier ist mein Programm:

public class Rational
{
    private final int numerator;
    private final int denominator;

    public Rational(int numerator, int denominator)
    {
        this.numerator = numerator;
        this.denominator = denominator;
    }
    public int getNumerator()
    {
        return numerator;
    }
    public int getDenominator()
    {
        return denominator;
    }
    public Rational swapSigns()
    {
        if      (numerator > 0 && denominator < 0)
        {
            return new Rational(-1*numerator,-1*denominator);
        }
        else if (numerator < 0 && denominator < 0)
        {
            return new Rational(-1*numerator,-1*denominator);
        }
        else
        {
            return new Rational(numerator,denominator);
        }   
    }
    public Rational inverse()
    {
        return new Rational(denominator,numerator);
    }
    public Rational simplify()
    {
        int gcd = Number.calculateGCDRecursively(Math.abs(numerator),denominator);
        return new Rational(numerator/gcd,denominator/gcd);
    }
    public Rational add(Rational otherRational)
    {
        otherRational = otherRational.swapSigns();
        int otherNumerator = otherRational.getNumerator();
        int otherDenominator = otherRational.getDenominator();
        int newDenominator = denominator*otherDenominator;
        int newNumerator = numerator*otherDenominator+denominator*otherNumerator;
        return new Rational(newNumerator,newDenominator).simplify();
    }
    public Rational subtract(Rational otherRational)
    {
        Rational oldRational = new Rational(numerator, denominator);
        int newNumerator = -1*otherRational.getNumerator();
        Rational newRational = new Rational(newNumerator,otherRational.getDenominator());
        return oldRational.add(newRational);
    }
    public Rational multipply(Rational otherRational)
    {
        otherRational = otherRational.swapSigns();
        int otherNumerator = otherRational.getNumerator();
        int otherDenominator = otherRational.getDenominator();
        int newNumerator = numerator*otherNumerator;
        int newDenominator = denominator*otherDenominator;
        return new Rational(newNumerator,newDenominator).simplify();
    }
    public Rational divide(Rational otherRational)
    {
        Rational oldRational = new Rational(numerator, denominator);
        Rational newRational = otherRational.inverse();
        return oldRational.multipply(newRational);
    }
    public String toString()
    {
        Rational oldRational = new Rational(numerator, denominator);
        oldRational = oldRational.swapSigns();
        return oldRational.getNumerator() + "/" + oldRational.getDenominator();
    }
    public static void main(String[] args)
    {
        int numerator1 = Integer.parseInt(args[0]);
        int denominator1 = Integer.parseInt(args[1]);
        int numerator2 = Integer.parseInt(args[2]);
        int denominator2 = Integer.parseInt(args[3]);
        Rational rational1 = new Rational(numerator1,denominator1);
        Rational rational2 = new Rational(numerator2,denominator2);
        System.out.println(rational1.toString() + " plus " + rational2.toString() + " is equal to " + rational1.add(rational2).toString());
        System.out.println(rational1.toString() + " minus " + rational2.toString() + " is equal to " + rational1.subtract(rational2).toString());
        System.out.println(rational1.toString() + " times " + rational2.toString() + " is equal to " + rational1.multipply(rational2).toString());
        System.out.println(rational1.toString() + " divided by " + rational2.toString() + " is equal to " + rational1.divide(rational2).toString());
    }
}

Außerdem habe ich die Methode calculateGCDRecursivelywie folgt geschrieben:

public static int calculateGCDRecursively(int p, int q)
{
    if (q == 0) return p;
    return calculateGCDRecursively(q, p % q);
}

Ich habe mein Programm überprüft und es funktioniert korrekt. Hier sind 4 verschiedene Beispiele dafür:

---

Input: 3 4 4 5
Output:
3/4 plus 4/5 is equal to 31/20
3/4 minus 4/5 is equal to -1/20
3/4 times 4/5 is equal to 3/5
3/4 divided by 4/5 is equal to 15/16

Input: 3 4 -4 5
Output:
3/4 plus -4/5 is equal to -1/20
3/4 minus -4/5 is equal to 31/20
3/4 times -4/5 is equal to -3/5
3/4 divided by -4/5 is equal to -15/16

Input: 3 4 4 -5
Output:
3/4 plus -4/5 is equal to -1/20
3/4 minus -4/5 is equal to 31/20
3/4 times -4/5 is equal to -3/5
3/4 divided by -4/5 is equal to -15/16

Input: 3 4 -4 -5
Output:
3/4 plus 4/5 is equal to 31/20
3/4 minus 4/5 is equal to -1/20
3/4 times 4/5 is equal to 3/5
3/4 divided by 4/5 is equal to 15/16

---

Gibt es eine Möglichkeit, mein Programm zu verbessern?

Danke für Ihre Aufmerksamkeit.

Answer 1

Wie immer schöne Umsetzung. Einige Vorschläge zum Code:

• Teilen durch 0 darf nicht erlaubt sein. Das Hinzufügen einer Prüfung im Konstruktor sollte ausreichen:

  public Rational(int numerator, int denominator) {
    if (denominator == 0) {
        throw new IllegalArgumentException("Denominator cannot be 0");
    }
    this.numerator = numerator;
    this.denominator = denominator;
  }
  

• Umkehren des Vorzeichens : Anstelle von können -1*numeratorSie verwenden -numerator. Es ist kompakter und immer noch lesbar.
• Tauschzeichen : Die Methode swapSignskann vereinfacht werden aus:

  public Rational swapSigns() {
      if (numerator > 0 && denominator < 0) {
          return new Rational(-1 * numerator, -1 * denominator);
      } else if (numerator < 0 && denominator < 0) {
          return new Rational(-1 * numerator, -1 * denominator);
      } else {
          return new Rational(numerator, denominator);
      }
  }
  

  Zu:

  private Rational swapSigns() {
    if (denominator < 0) {
        return new Rational(-numerator, -denominator);
    } else {
        return new Rational(numerator, denominator);
    }
  }
  

  Darüber hinaus swapSignsscheint nur intern von toStringund vor Operationen verwendet zu werden. Ich denke, es ist sinnvoll, die Zeichen im Konstruktor zu tauschen, zum Beispiel:

  public Rational(int numerator, int denominator) {
    if (denominator == 0) {
        throw new ArithmeticException("Denominator cannot be 0");
    }
    // Change 3/-4 to -3/4 or -3/-4 to 3/4
    this.numerator = denominator < 0 ? -numerator : numerator;
    this.denominator = denominator < 0 ? -denominator : denominator;
  }
  

  Damit swapSignkann das entfernt werden.
• Code Format : wie @ user985366 vorgeschlagen, formatieren Sie den Code , um es besser lesbar und übersichtlich zu machen. Moderne IDE wie Eclipse und IntelliJ bieten diese Funktion.
• Tippfehler in der Methode multipply.
• Rationalist unveränderlich : Dies bedeutet, dass keine temporären Objekte erstellt werden müssen wie oldRational:

  public Rational subtract(Rational otherRational){
      Rational oldRational = new Rational(numerator, denominator);
      int newNumerator = -1*otherRational.getNumerator();
      Rational newRational = new Rational(newNumerator,otherRational.getDenominator());
      return oldRational.add(newRational);
  }
  

  Kann vereinfacht werden zu:

  public Rational subtract(Rational otherRational) {
      int newNumerator = -otherRational.getNumerator();
      Rational newRational = new Rational(newNumerator, otherRational.getDenominator());
      return add(newRational);
  }
  

  Ähnliches gilt für multiply, divideund toString.

Testen

Das Testen im Wesentlichen ist keine gute Praxis, da das Programm von uns anhand der Ausgabe "getestet" werden muss. Es ist auch nicht ungewöhnlich, Anwendungen mit Hunderten oder Tausenden von Tests zu sehen. In diesem Fall wäre ein Testen im Wesentlichen nicht möglich.

Verwenden Sie Junit für Tests, einige Beispiele unten:

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

public class RationalTest {
    
    @Test
    public void testAddPositive() {
        Rational a = new Rational(3, 4);
        Rational b = new Rational(4, 5);

        Rational actual = a.add(b);

        Assertions.assertEquals(31, actual.getNumerator());
        Assertions.assertEquals(20, actual.getDenominator());
    }

    @Test
    public void testAddNegative() {
        Rational a = new Rational(3, 4);
        Rational b = new Rational(-4, 5);

        Rational actual = a.add(b);

        Assertions.assertEquals(-1, actual.getNumerator());
        Assertions.assertEquals(20, actual.getDenominator());
    }

    @Test
    public void testAddNegative2() {
        Rational a = new Rational(3, 4);
        Rational b = new Rational(4, -5);

        Rational actual = a.add(b);

        Assertions.assertEquals(-1, actual.getNumerator());
        Assertions.assertEquals(20, actual.getDenominator());
    }

    @Test
    public void testSubstract() {
        Rational a = new Rational(3, 4);
        Rational b = new Rational(4, 5);

        Rational actual = a.subtract(b);

        Assertions.assertEquals(-1, actual.getNumerator());
        Assertions.assertEquals(20, actual.getDenominator());
    }

    @Test
    public void testDivideBy0() {
        Assertions.assertThrows(IllegalArgumentException.class, () -> {
            new Rational(3, 0);
        }, "Denominator cannot be 0");
    }

    //... more tests
}

Überarbeitet Rational:

public class Rational {
    private final int numerator;
    private final int denominator;

    public Rational(int numerator, int denominator) {
        if (denominator == 0) {
            throw new IllegalArgumentException("Denominator cannot be 0");
        }
        // Change 3/-4 to -3/4 or -3/-4 to 3/4
        this.numerator = denominator < 0 ? -numerator : numerator;
        this.denominator = denominator < 0 ? -denominator : denominator;
    }

    public int getNumerator() {
        return numerator;
    }

    public int getDenominator() {
        return denominator;
    }

    public Rational inverse() {
        return new Rational(denominator, numerator);
    }

    public Rational simplify() {
        int gcd = Number.calculateGCDRecursively(Math.abs(numerator), denominator);
        return new Rational(numerator / gcd, denominator / gcd);
    }

    public Rational add(Rational otherRational) {
        int otherNumerator = otherRational.getNumerator();
        int otherDenominator = otherRational.getDenominator();
        int newDenominator = denominator * otherDenominator;
        int newNumerator = numerator * otherDenominator + denominator * otherNumerator;
        return new Rational(newNumerator, newDenominator).simplify();
    }

    public Rational subtract(Rational otherRational) {
        int newNumerator = -otherRational.getNumerator();
        Rational newRational = new Rational(newNumerator, otherRational.getDenominator());
        return add(newRational);
    }

    public Rational multiply(Rational otherRational) {
        int otherNumerator = otherRational.getNumerator();
        int otherDenominator = otherRational.getDenominator();
        int newNumerator = numerator * otherNumerator;
        int newDenominator = denominator * otherDenominator;
        return new Rational(newNumerator, newDenominator).simplify();
    }

    public Rational divide(Rational otherRational) {
        return multiply(otherRational.inverse());
    }

    public String toString() {
        return String.format("%d/%d", numerator, denominator);
    }
}

Answer 2

Zusätzlich zu Marc's Antwort:

Fügen Sie einige Testfälle hinzu, die eine Vereinfachung des Ergebnisses erfordern, z. B. 1/10 + 1/15 = 1/6.

Wenn Zähler oder Nenner ~ 50000 erreichen, überschreiten ihre Produkte (berechnet in Zwischenschritten) den ganzzahligen Bereich, selbst wenn das vereinfachte Ergebnis gut in diesen Bereich passt. Sie können dies verbessern, indem Sie longfür die Zwischenberechnungen den ganzzahligen Bereich prüfen, bevor Sie das neue Rational erstellen. Ihre aktuelle Version führt stillschweigend zu falschen Ergebnissen. In einem ArithmeticExceptionsolchen Fall wäre es besser, einen zu werfen .

Answer 3

Dies ist keine vollständige Antwort, nur ein paar Beobachtungen. Diese Antwort steht neben Marc 'ausgezeichneter Antwort und verfeinert seinen Code weiter.

Folgendes habe ich getan:

• Die größte Veränderung ist, dass ich mich class Rationalin eine verwandelt habe record. Die Reduzierung des Codes ist nicht ganz so dramatisch wie in einigen der künstlichen Beispiele, die Sie wahrscheinlich gesehen haben, da wir nicht den automatisch generierten toString()Konstruktor und den Konstruktor verwenden, aber dennoch werden wir die Getter und die Felddeklarationen los. Außerdem erhalten wir sinnvolle Implementierungen von hashCode()und equals()kostenlos!
• Ich habe simplify()in den Konstruktor eingefügt , so dass a Rationalimmer vereinfacht wird; keine Notwendigkeit zu nennen explizit simplify()in multiply()und add().
• Ich habe auch einige der lokalen Variablen in verschiedenen Methoden eingefügt.
• Ich habe einige Leerzeichen hinzugefügt, um dem Code mehr Raum zum Atmen zu geben und die "Schritte" in den Methoden visuell zu unterscheiden.
• Ich habe alles, was kann sein finaleigentlich final.
• Wo immer möglich, habe ich lokale Inferenztypen verwendet.
• Ich habe einige lokale Variablen und Methodenparameter umbenannt.
• Ich habe die calculateGcdRecursivelyMethode in just umbenannt gcd. Es interessiert niemanden, ob die Methode Rekursion, Iteration, einen Web-API-Aufruf, Magic Pixie Dust verwendet oder das Problem ausdrucken, an Malaysia faxen und von einem Mitarbeiter in einer Garage lösen lässt. Sie sollten sich auch nicht darum kümmern, und tatsächlich sollten sie es nicht wissen können. Darum geht es bei der Kapselung. Natürlich berechnet es auch die GCD. Was würde es sonst noch damit machen? Diese beiden Wörter sind überflüssig. Eigentlich ist das einzige Wort, das vielleicht buchstabiert werden sollte GreatestCommonDivisor, aber denken Sie daran, dass Code für ein Publikum geschrieben wurde, und ich glaube, dass die Art von Publikum, die die interne Implementierung von a liest, Rationalwissen sollte, was a gcdist.
• Ich @Overridehabe der toString()Methode aus gutem Grund eine Anmerkung hinzugefügt .
• Und einige kleinere Formatierungsänderungen.

Und so sieht das Ergebnis aus:

public record Rational(final int numerator, final int denominator) {
    public Rational(int numerator, int denominator) {
        if (denominator == 0) {
            throw new IllegalArgumentException("Denominator cannot be 0");
        }

        // Change 3/-4 to -3/4 or -3/-4 to 3/4
        numerator = denominator < 0 ? -numerator : numerator;
        denominator = denominator < 0 ? -denominator : denominator;

        final var gcd = gcd(Math.abs(numerator), denominator);

        this.numerator = numerator / gcd;
        this.denominator = denominator / gcd;
    }

    public Rational inverse() {
        return new Rational(denominator, numerator);
    }

    public Rational add(final Rational other) {
        final var otherNumerator = other.numerator;
        final var otherDenominator = other.denominator;

        final var newDenominator = denominator * otherDenominator;
        final var newNumerator = numerator * otherDenominator + denominator * otherNumerator;

        return new Rational(newNumerator, newDenominator);
    }

    public Rational subtract(final Rational other) {
        return add(new Rational(-other.numerator, other.denominator));
    }

    public Rational multiply(final Rational other) {
        final var otherNumerator = other.numerator;
        final var otherDenominator = other.denominator;

        final var newNumerator = numerator * otherNumerator;
        final var newDenominator = denominator * otherDenominator;

        return new Rational(newNumerator, newDenominator);
    }

    public Rational divide(final Rational other) {
        return multiply(other.inverse());
    }

    @Override public String toString() {
        return String.format("%d/%d", numerator, denominator);
    }

    private static int gcd(final int p, final int q) {
        if (q == 0) { return p; }
        return gcd(q, p % q);
    }
}

Ich persönlich finde das etwas leichter zu lesen, obwohl alles Lob an Marc gehen sollte. Ich gebe jedoch zu, dass fast alles stark auf Meinungen basiert. Außerdem sind Datensätze ein experimentelles Merkmal und können sich daher jederzeit ohne Vorwarnung inkompatibel ändern oder vollständig aus der Sprache entfernt werden. Außerdem benötigen sie hässliche Befehlszeilenoptionen, um zu funktionieren.