Eure Kritik: Fraction struct

Hallo,

ich wüsste gerne, welche Funktionalität ihr bei der Klasse Fraction (s. Anhang) noch vermisst ... Was könnte ich allgemein besser machen? Was ist unverständlich?
Sollte ich lieber IComparable<Fraction> verwenden? Wenn ja, warum?

m0rius

Hallo m0rius,

Sollte ich lieber IComparable<Fraction> verwenden?

Warum lieber? Mit Brüchen kann beides machen. Also solltest du auch beide Interfaces implementieren.

Was könnte ich allgemein besser machen? *Fraction ist ein klassicher Werttyp und sollte daher ein struct sein *Clone ist dann überflüssig/störend *AutoReduce als Property ohne zugehöriges Feld kommt mir komisch vor. *GreatestCommonDivisor/LeastCommonMultiple beziehen sich vermutlich auf Zähler und Nenner und nicht auf den Bruch als Ganzes und passen daher eigentlich nicht. *IsGreaterThen/IsLowerThen gefällt mir nicht. Gerade wenn IComparable<Fraction> implementiert ist, ist es redundant. Außerdem hast du ja die passenden Operatoren überschrieben. *Für ToString könnte es Überladungen für Formatstrings/Formatpovider geben.

herbivore

Hallo herbivore,

danke erstmal für Dein Feedback!

Sollte ich also die Funktionen GCD bzw. LCM in eine Klasse FractionHelper o.ä. auslagern? Diese werden ja zum Kürzen und für andere Rechenarten gebraucht ...

Würdest Du IsGreaterThan() und IsLowerThan wirklich rausnehmen?
Momentan werden diese Funktionen nämlich von CompareTo() und den vier überladenen Operatoren <, >, ≤ und ≥ bemüht ... Designfehler?

m0rius

Eine Überschreibung von GetHashCode() sollte rein, besonders wichtig als Struct oder Key in einem Dictionary.

Würdest Du IsGreaterThan() und IsLowerThan wirklich rausnehmen?
Momentan werden diese Funktionen nämlich von CompareTo() und den vier überladenen Operatoren <, >, ≤ und ≥ bemüht ... Designfehler?

Dann mach sie private.

Hallo kleines_eichhoernchen,

die Methoden sind jetzt private. Ich würde trotzdem gern wissen, weswegen sie von euch beiden als so redundant beschrieben wurden ...
Ich kann Additionen ja auch mit dem überladenen Operator vornehmen; müsste ich dann nicht konsequenterweise auch Add() usw. als private markieren?

GetHashCode() hatte ich überschrieben, allerdings erst nach meinem ersten Post.

public override int GetHashCode()
{
    Fraction fraction = new Fraction(this);
    fraction.Reduce();

    return fraction.Numerator + fraction.Denominator;
}

Ist die Implementation so in Ordnung?

Inwiefern wäre die Einführung einer Enumeration mit den Zuständen Sign.Positive und Sign.Negative sinnvoll?

m0rius

Hallo m0rius,

Sollte ich also die Funktionen GCD bzw. LCM in eine Klasse FractionHelper o.ä. auslagern? Diese werden ja zum Kürzen und für andere Rechenarten gebraucht ...

eine extra Klasse brauchst du nicht. Es reicht, wenn die Methoden private (und static) sind.

Würdest Du IsGreaterThan() und IsLowerThan wirklich rausnehmen?

Ja, denn es gibt ja schon CompareTo.

Momentan werden diese Funktionen nämlich von CompareTo() und den vier überladenen Operatoren <, >, ≤ und ≥ bemüht ... Designfehler?

Ich würde die Operatoren mittels CompareTo implementieren.

Ich kann Additionen ja auch mit dem überladenen Operator vornehmen; müsste ich dann nicht konsequenterweise auch Add() usw. als private markieren?

Ja, diese Redundanzen sind mir bewusst. Ich habe nichts dagegen gesagt, weil diese Redundanzen auch bei DateTime vorkommen.

Ist die Implementation so in Ordnung?

Falsch ist die nicht, aber es gibt bessere. Verwende besser eine Verknüpfung, die nicht kommutativ ist.

Inwiefern wäre die Einführung einer Enumeration mit den Zuständen Sign.Positive und Sign.Negative sinnvoll?

Kann man machen. Bringt mich auf die Idee, dass du noch eine Methode MormalizeSige o.ä. brauchen könntest, die dafür sorgt, dass nur der Zähler ein Vorzeichen hat, aber der Nenner keins.

herbivore

Hallo m0rius,

du könntest auch den Typ Decimal näher betrachten (statische Operatoren, Konstanten, sinnvolle Methoden).

Spooky

Hallo zusammen,

für diejenigen, die es interessiert: Ich habe jetzt die in diesem Post vorgeschlagene Verknüpfung für GetHashCode() verwendet.

Hallo herbivore,

die Implementierung der Operatoren mittels CompareTo() ist tatsächlich viel komfortabler (und auch kürzer), danke für den Hinweis.

m0rius

Ich kann Additionen ja auch mit dem überladenen Operator vornehmen; müsste ich dann nicht konsequenterweise auch Add() usw. als private markieren?

Überladene Operatoren sollte immer auch eine benamte Alternativmethode haben, da es .NET Sprachen gibt, die überladene Operatoren nicht unterstützen.
Siehe Operator overloades have named alternates

Gruß Jake

Hallo Jake,

daher meine Überlegung mit den Methoden IsLowerThan() und IsGreaterThan(), wobei diese allerdings mit entsprechender Auswertung durch CompareTo() ersetzt werden können (s. herbivores Post).

m0rius