Implementación de tipo de datos para números racionales

Question

Este es el ejercicio 3.2.7. del libro Computer Science An Interdiscipcise Approach de Sedgewick & Wayne:

Implemente un tipo de datos para números racionales que admita sumas, restas, multiplicaciones y divisiones.

Aquí está mi programa:

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());
    }
}

También escribí el método de la calculateGCDRecursivelysiguiente manera:

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

Revisé mi programa y funciona correctamente. Aquí hay 4 instancias diferentes de esto:

---

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

---

¿Hay alguna forma de mejorar mi programa?

Gracias por tu atención.

Answer 1

Como siempre, buena implementación. Algunas sugerencias sobre el código:

• No se debe permitir la división por 0 . Agregar un cheque en el constructor debería ser suficiente:

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

• Invierta el signo : en lugar de -1*numeratorpuede usar -numerator. Es más compacto y aún legible.
• Signo de intercambio : el método swapSignsse puede simplificar de:

  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);
      }
  }
  

  A:

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

  Además, swapSignsparece ser utilizado solo internamente por toStringy antes de cualquier operación. Creo que tiene sentido "intercambiar los signos" en el constructor, por ejemplo:

  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;
  }
  

  Entonces eso swapSignse puede quitar.
• Formato de código : como sugirió @ user985366, formatee el código para hacerlo más legible y conciso. Los IDE modernos como Eclipse e IntelliJ proporcionan esa función.
• Error tipográfico en el método multipply.
• Rationales inmutable : lo que significa que no es necesario crear objetos temporales como 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);
  }
  

  Puede simplificarse a:

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

  Del mismo modo para multiply, dividey toString.

Pruebas

Probar en general no es una buena práctica, porque el programa debe ser "probado" por nosotros mirando el resultado. Además, no es raro ver aplicaciones con cientos o miles de pruebas. En ese caso, la prueba en general sería inviable.

Utilice Junit para las pruebas, algunos ejemplos a continuación:

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
}

Refactorizado 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

Además de la respuesta de Marc:

Agregue algunos casos de prueba que necesiten simplificar el resultado, por ejemplo, 1/10 + 1/15 = 1/6.

Si los numeradores o denominadores alcanzan ~ 50000, sus productos (calculados en pasos intermedios) excederán el rango de números enteros, incluso si el resultado simplificado encaja bien en ese rango. Puede mejorarlo utilizando longpara los cálculos intermedios y comprobar el rango de enteros antes de crear el nuevo Rational. Su versión actual producirá silenciosamente resultados incorrectos. Lanzar un ArithmeticExceptionen tal caso sería mejor.

Answer 3

Esta no es una respuesta completa, solo un par de observaciones. Esta respuesta se suma a la excelente respuesta de Marc y refina aún más su código.

Esto es lo que hice:

• El mayor cambio es que me cambié class Rationala record. La reducción en el código no es tan dramática como en algunos de los ejemplos artificiales que probablemente haya visto, ya que no estamos usando el toString()constructor y autogenerado , pero aún así, nos deshacemos de los captadores y las declaraciones de campo. Además, ¡obtenemos implementaciones sensatas de hashCode()y equals()gratis!
• Incluí simplify()en el constructor, de modo que a Rationalsiempre se simplifica; no hay necesidad de llamar de forma explícita simplify()en multiply()y add().
• También incluí algunas de las variables locales en varios métodos.
• Agregué algunos espacios en blanco para darle al código más espacio para respirar y distinguir visualmente los "pasos" en los métodos.
• Hice todo lo que puede ser en finalrealidad final.
• Usé la inferencia de tipo de variable local siempre que fue posible.
• Cambié el nombre de algunas variables locales y parámetros de métodos.
• Cambié el nombre del calculateGcdRecursivelymétodo a solo gcd. A nadie le importa si el método utiliza recursividad, iteración, una llamada a la API web, polvo mágico de duendes, o imprime el problema, lo envía por fax a Malasia y tiene un tipo en un garaje que lo resuelva. Tampoco debería importarles y, de hecho, no deberían poder saberlo. De eso se trata la encapsulación. Además, por supuesto, calcula el GCD, ¿qué más haría con él? Esas dos palabras son redundantes. Realmente, la única palabra que quizás debería escribirse es GreatestCommonDivisor, pero recuerde que el código está escrito para una audiencia, y creo que el tipo de audiencia que lee la implementación interna de a Rationaldebería saber qué gcdes a.
• Agregué una @Overrideanotación al toString()método, por si acaso.
• Y algunos cambios de formato menores.

Y así es como se ve el resultado:

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);
    }
}

Personalmente, encuentro esto algo más fácil de leer, aunque todos los elogios deben ir para Marc. Sin embargo, admito que casi todo está basado en opiniones. Además, los registros son una función experimental y, por lo tanto, pueden cambiar de manera incompatible o eliminarse completamente del idioma en cualquier momento sin previo aviso. Además, requieren opciones de línea de comando feas para funcionar.