Часть кода

Следуйте за этим сообщением и включите некоторые основные предлагаемые варианты , вот пересмотренный код.

Изменения включают:

  • Больше не хранить IFormatProvider на уровне экземпляра.
  • Удалены параметры конструктора IFormatProvider.
  • Введена перегрузка ToString(IFormatProvider).
  • Изменен decimal до float, для использования float.NaN для дробей с 0 знаменателями.
  • Реализовано больше операторов.
  • Reimplemented Equals и CompareTo за рекомендации.
  • Исправлена ​​ошибка в ToString(string, IFormatProvider).
  • Добавлены комментарии XML для всех публичных пользователей.

По мере того, как файл кода рос, стало очевидно, что мне понадобится способ группировки разделов кода. Я не хотел использовать #region, потому что ... это вопрос принципов. Это иррационально, я просто не хочу использовать #region.

Итак, я перегруппировал все члены static в partial struct (обратите внимание: некоторые строки кода и комментарии XML были переформатированы, чтобы избежать горизонтальной прокрутки):

/// <summary>
/// A fractional representation of a rational number.
/// </summary>
public partial struct Fraction
{
    /// <summary>
    /// An empty <c>Fraction</c> (0/0).
    /// </summary>
    public static readonly Fraction Empty = new Fraction();

    /// <summary>
    /// A <c>Fraction</c> representation of the integer value 0.
    /// </summary>
    public static readonly Fraction Zero = new Fraction(default(int));

    /// <summary>
    /// A <c>Fraction</c> representation of the integer value 1.
    /// </summary>
    public static readonly Fraction One = new Fraction(1);

    /// <summary>
    /// Represents the smallest possible value for a <see cref="Fraction"/>.
    /// </summary>
    public static readonly Fraction MinValue = new Fraction(1, int.MinValue);

    /// <summary>
    /// Represents the largest possible value for a <see cref="Fraction"/>.
    /// </summary>
    public static readonly Fraction MaxValue = new Fraction(int.MaxValue, 1);

    /// <summary>
    /// Returns a simplified/reduced representation of a fraction.
    /// </summary>
    /// <param name="fraction">The fraction to simplify.</param>
    /// <returns>
    /// Returns a new <see cref="Fraction"/>,
    /// a simplified representation of this instance (if simplification is possible).
    /// </returns>
    public static Fraction Simplify(Fraction fraction)
    {
        if (fraction.IsUndefined)
        {
            return new Fraction(fraction);
        }

        var gcd= GetGreatestCommonDenominator(fraction.Numerator, fraction.Denominator);

        var numerator = fraction.Numerator / gcd;
        var denominator = fraction.Denominator / gcd;

        return new Fraction(numerator, denominator);
    }

    private static int GetGreatestCommonDenominator(int numerator, int denominator)
    {
        return denominator == 0 
                   ? numerator
                   : GetGreatestCommonDenominator(denominator, numerator % denominator);
    }

    private static readonly Regex _parserRegex = 
        new Regex(@"^\s*?(?<numerator>\d+)\s*?/\s*?(?<denominator>\d+)\s*?$");

    /// <summary>
    /// Converts the string representation of a fraction 
    /// into its <c>Fraction</c> equivalent.
    /// A return value indicates whether the conversion succeeded.
    /// </summary>
    /// <param name="s">A string containing the fraction to convert.</param>
    /// <param name="result">
    /// When this method returns, contains the <c>Fraction</c> 
    /// equivalent to the specified string.
    /// </param>
    /// <returns>Returns <c>true</c> if conversion is successful.</returns>
    public static bool TryParse(string s, out Fraction result)
    {
        var syntaxMatch = _parserRegex.Match(s);
        if (!syntaxMatch.Success)
        {
            result = Fraction.Zero;
            return false;
        }

        var numerator = int.Parse(syntaxMatch.Groups["numerator"].Value);
        var denominator = int.Parse(syntaxMatch.Groups["denominator"].Value);

        result = new Fraction(numerator, denominator);
        if (!result.IsUndefined)
        {
            result = result.Simplify();
        }

        return true;
    }

    public static explicit operator float(Fraction fraction)
    {
        return fraction.ToFloat();
    }

    public static bool operator ==(Fraction fraction1, Fraction fraction2)
    {
        return fraction1.Equals(fraction2);
    }

    public static bool operator ==(Fraction fraction, int value)
    {
        return fraction.Equals(new Fraction(value));
    }

    public static bool operator ==(Fraction fraction, float value)
    {
        Fraction result;
        if (Fraction.TryParse(value.ToString(), out result))
        {
            return fraction.Equals(result);
        }

        return false;
    }

    public static bool operator !=(Fraction fraction1, Fraction fraction2)
    {
        return !(fraction1 == fraction2);
    }

    public static bool operator !=(Fraction fraction, int value)
    {
        return !(fraction == value);
    }

    public static bool operator !=(Fraction fraction, float value)
    {
        return !(fraction == value);
    }

    public static Fraction operator ++(Fraction fraction)
    {
        return new Fraction(fraction.Numerator + 1, fraction.Denominator);
    }

    public static Fraction operator +(Fraction fraction, int value)
    {
        return fraction + new Fraction(value);
    }

    public static Fraction operator +(Fraction fraction1, Fraction fraction2)
    {
        int numerator = (fraction1.Numerator * fraction2.Denominator)
                      + (fraction1.Denominator * fraction2.Numerator);
        int denominator = (fraction1.Denominator * fraction2.Denominator);

        var result = new Fraction(numerator, denominator).Simplify();
        return result;
    }

    public static Fraction operator --(Fraction fraction)
    {
        return new Fraction(fraction.Numerator - 1, fraction.Denominator);
    }

    public static Fraction operator -(Fraction fraction, int integer)
    {
        return fraction - new Fraction(integer);
    }

    public static Fraction operator -(Fraction fraction1, Fraction fraction2)
    {
        var subtrator = new Fraction(fraction2.Numerator * -1, fraction2.Denominator);
        return fraction1 + subtrator;
    }

    public static Fraction operator /(Fraction fraction, int integer)
    {
        return fraction / new Fraction(integer);
    }

    public static Fraction operator /(Fraction fraction1, Fraction fraction2)
    {
        var divisor = new Fraction(fraction2.Denominator, fraction2.Numerator);
        return fraction1 * divisor;
    }

    public static Fraction operator *(Fraction fraction, int integer)
    {
        return fraction * new Fraction(integer);
    }

    public static Fraction operator *(Fraction fraction1, Fraction fraction2)
    {
        var numerator = fraction1.Numerator * fraction2.Numerator;
        var denominator = fraction1.Denominator * fraction2.Denominator;

        var result = new Fraction(numerator, denominator).Simplify();
        return result;
    }
}

Это оставило все члены экземпляра в собственном файле:

/// <summary>
/// A fractional representation of a rational number.
/// </summary>
[Serializable]
public partial struct Fraction : IFormattable,
                                 IComparable, 
                                 IComparable<Fraction>,
                                 IEquatable<Fraction>
{
    private readonly int _numerator;
    private readonly int _denominator;

    /// <summary>
    /// Copy constructor. 
    /// Creates a new <c>Fraction</c> instance based on the specified value.
    /// </summary>
    /// <param name="fraction"></param>
    public Fraction(Fraction fraction)
        : this(fraction.Numerator, fraction.Denominator)
    {
    }

    /// <summary>
    /// Creates a new <c>Fraction</c> with the denominator being 1.
    /// </summary>
    /// <param name="numerator"></param>
    public Fraction(int numerator)
        : this(numerator, 1)
    {
    }

    /// <summary>
    /// Creates a new <c>Fraction</c> with specified numerator and denominator.
    /// </summary>
    /// <param name="numerator"></param>
    /// <param name="denominator"></param>
    public Fraction(int numerator, int denominator)
    {
        _numerator = numerator;
        _denominator = denominator;
    }

    /// <summary>
    /// Gets the numerator (get-only).
    /// </summary>
    public int Numerator { get { return _numerator; } }

    /// <summary>
    /// Gets the denominator (get-only).
    /// </summary>
    public int Denominator { get { return _denominator; } }

    /// <summary>
    /// Gets a value indicating whether this instance is defined.
    /// Returns true when the fraction is a division by zero.
    /// </summary>
    public bool IsUndefined { get { return _denominator == default(int); } }

    /// <summary>
    /// Simplifies/reduces the fraction.
    /// </summary>
    /// <returns>
    /// Returns a simplified representation of this instance, 
    /// if simplification is possible.
    /// </returns>
    public Fraction Simplify()
    {
        return Fraction.Simplify(this);
    }

    /// <summary>
    /// Creates a <c>float</c> representation of the <see cref="Fraction"/>.
    /// </summary>
    /// <returns>
    /// Returns the result of dividing the <c>Numerator</c> by the <c>Denominator</c>, 
    /// or <c>float.NaN</c> when the <c>Denominator</c> is zero.
    /// </returns>
    public float ToFloat()
    {
        return IsUndefined ? float.NaN 
                           : (float)_numerator / (float)_denominator;
    }

    /// <summary>
    /// Returns a value indicating whether this instance and a specified object 
    /// represent the same value.
    /// </summary>
    /// <param name="obj">Any <c>Fraction</c> or <c>float</c>-convertible value.</param>
    /// <returns></returns>
    public override bool Equals(object obj)
    {
        if (obj is Fraction)
        {
            return Equals((Fraction)obj);
        }

        return ToFloat().Equals((float)obj);
    } 

    /// <summary>
    /// Returns the hash code for this instance.
    /// </summary>
    /// <returns></returns>
    public override int GetHashCode()
    {
        return ToFloat().GetHashCode();
    }

    /// <summary>
    /// Converts this fraction into a string representation, 
    /// using a default <see cref="FractionFormatter"/>.
    /// </summary>
    /// <returns></returns>
    public override string ToString()
    {
        return ToString(FractionFormatter.Default);
    }

    /// <summary>
    /// Converts this fraction into a string representation,
    /// using specified <c>IFormatProvider</c>.
    /// </summary>
    /// <param name="provider"></param>
    /// <returns></returns>
    public string ToString(IFormatProvider provider)
    {
        return ToString(null, provider);
    }

    /// <summary>
    /// Converts this fraction into a string representation,
    /// using specified <c>format</c> and <c>IFormatProvider</c>.
    /// </summary>
    /// <param name="format"></param>
    /// <param name="provider"></param>
    /// <returns></returns>
    public string ToString(string format, IFormatProvider provider)
    {
        if (provider is ICustomFormatter)
        {
            return ((ICustomFormatter)provider).Format(format, this, provider);
        }

        return FractionFormatter.Default.Format(format, this, FractionFormatter.Default);
    }

    /// <summary>
    /// Compares this instance to a specified object and 
    /// returns an indication of their relative values.
    /// </summary>
    /// <param name="obj"></param>
    /// <returns></returns>
    public int CompareTo(object obj)
    {
        if (obj is int)
        {
            return CompareTo(new Fraction((int)obj));
        }
        else if (obj is string)
        {
            Fraction fraction;
            if (Fraction.TryParse(obj as string, out fraction))
            {
                return CompareTo(fraction);
            }
        }

        return CompareTo((Fraction)obj);
    }

    /// <summary>
    /// Compares this instance to specified <c>Fraction</c> and
    /// returns an indication of their relative values.
    /// </summary>
    /// <param name="other"></param>
    /// <returns></returns>
    public int CompareTo(Fraction other)
    {
        if (IsUndefined || other.IsUndefined)
        {
            // let the framework handle NaN comparisons
            return ToFloat().CompareTo(other.ToFloat());
        }

        long left = _numerator * other.Denominator;
        long right = _denominator * other.Numerator;

        return left.CompareTo(right);
    }

    /// <summary>
    /// Returns a value indicating whether 
    /// this instance is equal to a specified <c>Fraction</c> value.
    /// </summary>
    /// <param name="other"></param>
    /// <returns></returns>
    public bool Equals(Fraction other)
    {
        return CompareTo(other) == 0;
    }
}

Вот скриншот представления Solution Explorer , показывающего всех участников и перегрузки:

Члены типа фракции

Все ли согласовано? Что можно улучшить? @mjolka предложил написать GetGreatestCommonDenominator итеративно - я нашел этот код онлайн, и я считаю, что рекурсивный метод легче читать:

static int GetGreatestCommonDenominator(int numerator, int denominator)
{
    int remainder;

    while (denominator != 0)
    {
        remainder = numerator % denominator;
        numerator = denominator;
        denominator = remainder;
    }

    return numerator;
}

Я жертвую чем-то здесь?


FractionFormatter также претерпел некоторые незначительные изменения, включая его здесь для завершения:

public class FractionFormatter : IFormatProvider, ICustomFormatter
{
    private readonly CultureInfo _culture;

    public FractionFormatter(CultureInfo culture)
    {
        _culture = culture;
    }

    public static FractionFormatter Default
    {
        get { return new FractionFormatter(CultureInfo.CurrentUICulture); }
    }

    public object GetFormat(Type formatType)
    {
        return (formatType == typeof(ICustomFormatter)) ? this : null;
    }

    public string Format(string format, object arg, IFormatProvider formatProvider)
    {
        var fraction = (Fraction)arg;
        if (string.IsNullOrEmpty(format))
        {
            return string.Format(_culture, "{0}/{1}", fraction.Numerator, fraction.Denominator);
        }

        var result = string.Format(_culture, "{0:" + format + "}", fraction.ToFloat());
        return result;
    }
}

То же самое с MathJaxFractionFormatter:

public class MathJaxFractionFormatter : IFormatProvider, ICustomFormatter
{
    public enum MathJaxFractionSize
    {
        Normal,
        Large
    }

    private static readonly CultureInfo _culture = typeof(FractionFormatter).Assembly.GetName().CultureInfo;

    private readonly string _delimiter;
    private readonly MathJaxFractionSize _size;

    public MathJaxFractionFormatter()
        : this("$", MathJaxFractionSize.Normal) { }

    public MathJaxFractionFormatter(string delimiter, MathJaxFractionSize size)
    {
        _delimiter = delimiter;
        _size = size;
    }

    public object GetFormat(Type formatType)
    {
        return (formatType == typeof(ICustomFormatter)) ? this : null;
    }

    public string Format(string format, object arg, IFormatProvider formatProvider)
    {
        var fraction = (Fraction)arg;
        if (string.IsNullOrEmpty(format))
        {
            var keyword = _size == MathJaxFractionSize.Normal ? "\\frac" : "\\dfrac";
            return string.Format(_culture, "{2}{3}{{{0}}}{{{1}}}{2}", fraction.Numerator, fraction.Denominator, _delimiter, keyword);
        }

        return fraction.ToString(format, _culture);
    }
}

Теперь это напечатает чувствительный к культуре 33.333 %:

Console.WriteLine(new Fraction(1, 3).ToString("p3", new MathJaxFractionFormatter()));

И это напечатает $\frac{1}{3}$, как ожидалось:

Console.WriteLine(new Fraction(1, 3).ToString(new MathJaxFractionFormatter()));

Передача в CultureInfo.InvariantCulture приведет просто к ToString для использования FractionFormatter.Default.

Console.WriteLine("5/25 with InvariantCulture: {0}", 
                  new Fraction(5, 25).ToString(CultureInfo.InvariantCulture));

Вывод 5/25 with InvariantCulture: 5/25.

11 голосов | спросил Mathieu Guindon 14 J000000Monday14 2014, 08:23:51

2 ответа


11

Я бы добавил комментарии mjolka о том, что поведение ++ и -- вводит в заблуждение и что 0 предпочтительнее default(int)

NaN и бесконечность

У вас есть один случай для нулевого знаменателя, называемый IsUndefined. Для сравнения, Single имеет IsInfinity, ---- +: = 7 =: + ----, IsPositiveInfinity и IsNegativeInfinity. Аналогично, он имеет статический IsNaN, NaN и NegativeInfinity.

Для согласованности я бы подумал о переименовании PositiveInfinity в Empty и разбиение NaN на три случая, для числителя <0 (отрицательная бесконечность), == 0 (NaN) и> 0 (положительная бесконечность). Они могут использоваться при преобразовании в плав и обратно.

Синтаксический

Вам следует использовать IsUndefined, а не int.TryParse. Например, если числитель или знаменатель больше, чем int.Parse, вы хотите вернуть false, а не бросать исключение.

int.Max (и аналогичные методы) фактически гарантируют конкретное значение их выходному параметру. Трудно думать о ситуации, когда это важно, но если вы хотите быть супер-conscietious, вы можете подумать о том, чтобы сделать то же самое. Обратите внимание: в отличие от большинства числовых типов In32.TryParse на самом деле не default(Fraction), но Fraction.Zero. Возможно, вам захочется подумать о том, из какого из них вы скорее вернетесь - 0/1 для согласованности с другими числовыми типами, или 0/0 для согласованности с Fraction.Empty. И в любом случае вы можете четко указать свое решение в своих комментариях xml.

Вы также можете подумать о методе default, таком как другие числовые типы. Это не важно, но это экономит потребителю некоторую работу в общей ситуации, когда недопустимый ввод считается исключительным.

Облегчение

Кажется немного непредсказуемым, когда фракции упрощаются. Я бы не ожидал, что я могу определить долю 2/4, и это останется таким, пока я не умножу его на 1, после чего он станет 1/2. Особенно, делая публичный Parse, я думаю, вы подразумеваете, что упрощение - это то, что должно работать понятным для потребителя способом, без необходимости читать все ваши реализации методов.

Вы можете пройти и попытаться использовать его последовательно, но я думаю, что самым простым способом было бы упрощение внутри конструктора, чтобы гарантировать, что все дроби всегда будут упрощены. Я не могу представить убедительной причины, по которой вы хотите работать с неудовлетворительными фракциями. В этом случае вы хотели бы сделать методы, связанные с упрощением private. И, очевидно, им нужно было бы взять числитель и знаменатель в качестве параметров, а не фракций.

Избегаемые переполнения

Это, возможно, относится к категории «хорошо иметь», но вы должны попытаться обеспечить, чтобы вы не попадали в переполнения внутри арифметических операций, когда этого избежать. Например, если у вас есть Simplify, определенный как int.Max /2 + 1, тогда a /2 * 2 /a будет переполняться, когда он должен равняться 1/1.

Для умножения, если у вас есть /b * c /d, оба уже упрощены, то самым простым способом сделать это было бы сначала определить и упростить a /d и c /b, а затем умножить их.

Я не думаю, что есть какие-либо улучшения, которые вы можете сделать для добавления, пока входные дроби уже упрощены. EDIT: На самом деле, одна вещь, которую вы можете сделать, - это не использовать продукт двух знаменателей, вместо этого используйтенаименьшее общее кратное. Вам нужно будет разобраться, что нужно умножить на два числителя, прежде чем добавлять их. На самом деле, это, вероятно, более важно, чем умножение, потому что даже добавление 1/50000 + 1/50000 будет переполняться, как сейчас.

Сравнение

Согласно документации msdn для a:

  

Параметр obj должен быть того же типа, что и класс или тип значения, который реализует этот интерфейс; в противном случае генерируется исключение ArgumentException.

Я могу понять, почему вы можете поддерживать сравнение с целым числом, но по сравнению со строкой действительно кажется излишним. Это также потенциально опасно, поскольку оно не соответствует всем этим правилам из той же статьи:

  

A.CompareTo (A) должен возвращать ноль.

     

Если A.CompareTo (B) возвращает ноль, тогда B.CompareTo (A) должен вернуть ноль.

     

Если A.CompareTo (B) возвращает ноль, а B.CompareTo (C) возвращает ноль, то A.CompareTo (C) должен возвращать ноль.

     

Если A.CompareTo (B) возвращает значение, отличное от нуля, тогда B.CompareTo (A) должно вернуть значение противоположного знака.

     

Если A.CompareTo (B) возвращает значение x, не равное нулю, а B.CompareTo (C) возвращает значение y того же знака, что и x, то A.CompareTo (C) должно вернуть значение тот же знак, что и x и y.

Так как строки сравниваются в алфавитном порядке, легко представить себе комбинацию из двух строк и фракции, которая будет сравнивать непоследовательно. например:

IComparable.CompareTo

GetHashCode

Как бы то ни было, я мог бы получить два экземпляра, где var quarter = "1/4"; var third = new Fraction(1,3); var half = "1/2"; Console.Out.WriteLine(quarter.CompareTo(half)); // 1 Console.Out.WriteLine(third.CompareTo(half)); // -1 Console.Out.WriteLine(third.CompareTo(quarter)); // 1 возвращает true, но имеет другой хеш-код из-за плавающее округление. Я сделал быстрый тест, и я считаю, что 1/3 будет иметь другой хэш-код для 5592409/16777227, например.

Обеспечение того, чтобы все дроби всегда были упрощены, как я описал ранее, это исправит. В качестве альтернативы вы можете использовать Equals, хотя я не уверен в влиянии производительности. Вы также можете изучить формулы для генерации хэш-кодов из двух целых чисел. Или, для явной согласованности с методом Tuple.Create(_numerator, _denominator).GetHashCode(), вы можете найти Equals произведение числителя и знаменателя и взять хэш-код этого.

long

Для дополнительной поддержки вы можете рассмотреть возможность использования IConvertable . Это относительно просто и, вероятно, лучше всего показано в связанной статье. Для определенных типов, таких как IConvertable, Int32 и Int64, вы можете захотеть реализовать свое собственное преобразование, но в противном случае самый простой способ - преобразовать сначала в общий числовой тип (вероятно, ---- +: = 38 =: + ----), затем вызовите соответствующий метод Boolean результат.

ответил Ben Aaronson 14 J000000Monday14 2014, 16:15:37
11

Это хорошо! Вот некоторые проблемы, которые я нашел.

читаемость

Я бы порекомендовал 0 over default(int) для удобства чтения.

Нейминг

Самый большой общий делитель является более стандартным термином для наибольший общий знаменатель .

MinValue

Fraction.MinValue должен быть new Fraction(int.MinValue, 1), не new Fraction(1, int.MinValue).

Неизменность

Поскольку ваша структура данных неизменна, этот код

public static Fraction Simplify(Fraction fraction)
{
    if (fraction.IsUndefined)
    {
        return new Fraction(fraction);
    }
    ...

может быть

public static Fraction Simplify(Fraction fraction)
{
    if (fraction.IsUndefined)
    {
        return fraction;
    }
    ...

++ и --

Из MSDN :

  

Оператор приращений (++) увеличивает свой операнд на 1.

Однако, как записываются операторы приращения и декремента, они увеличивают (уменьшают) на 1 / denominator. Это приводит к запутанному поведению.

Например, как мы можем сделать это утверждение неудачным?

if (x == y)
{
    x++;
    y++;
    Debug.Assert(x == y);
}

Просто установите

var x = new Fraction(2, 4);
var y = x.Simplify();

После инкремента мы имеем x = 3/4 и y = 2/2

Или этот надуманный пример не сработает для x = new Fraction(1, 2):

var y = x;
// Waste time by doing nothing to x.
x++;
x *= 1;
x--;
// Make sure we didn't change x.
Debug.Assert(x == y);

Я бы придерживался руководящих принципов и увеличивал (уменьшался) на 1.

TryParse

Возможно, вы захотите использовать класс символов [0-9] вместо \d в вашем регулярном выражении. Это связано с тем, что \d соответствует десятичным разрядам Unicode. Таким образом, этот код выведет код FormatException:

Fraction f;
if (Fraction.TryParse("١/٢", out f))
{
    Console.WriteLine(f);
}

Поплавки

Сравнение с поплавками ... сложно.

float oneThird = 1.0f / 3;
Console.WriteLine(new Fraction(1, 3) == oneThird);
> False

(О, и хороший звонок не используется #region .)

ответил mjolka 14 J000000Monday14 2014, 09:01:42

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132