Усилия в итерации - FizzBuzz - PullRequest
       14

Усилия в итерации - FizzBuzz

0 голосов
/ 14 апреля 2011

РЕДАКТИРОВАТЬ За то, что это стоит, что может быть не так уж много. Я сделал небольшой тест, чтобы расширить этот вопрос.

Я написал две функции для перечисления серии FizzBuzz.

private static IEnumerable<string> SimpleFizzBuzz(
        int start = 0,
        int end = int.MaxValue)
{
    return Enumerable.Range(start, end).Select(i =>
        i % 15 == 0 ? "fizzbuzz" :
        i % 3 == 0 ? "fizz" :
        i % 5 == 0 ? "buzz" :
        i.ToString(CultureInfo.InvariantCulture));
}

и

private static IEnumerable<string> OptimizedFizzBuzz(
        int start = 0,
        int end = int.MaxValue)
{
    const int fizz = 3;
    const int buzz = 5;
    const string fizzString = "fizz";
    const string buzzString = "buzz";
    const string fizzBuzzString = fizzString + buzzString;

    var fizzer = start % fizz;
    var buzzer = start % buzz;

    if (fizzer == 0)
    {
        fizzer = fizz;
    }

    if (buzzer == 0)
    {
        buzzer = buzz;
    }

    for (var i = start; i <= end; i++)
    {
        if (buzzer == buzz)
        {
            if (fizzer == fizz)
            {
                yield return fizzBuzzString;
                buzzer = 1;
                fizzer = 1;
                continue;
            }

            yield return buzzString;
            buzzer = 1;
            fizzer++;
            continue;
        }

        if (fizzer == fizz)
        {
            yield return fizzString;
            buzzer++;
            fizzer = 1;
            continue;
        }

        yield return i.ToString(CultureInfo.InvariantCulture);
        fizzer++;
        buzzer++;
    }
}

Я немного поработал, скомпилировал в конфигурации выпуска, с оптимизацией и запустил из командной строки. За 10^8 итераций, без лишних затрат, связанных с отчетом о каждом элементе, я получаю результаты, приблизительно равные

Простой: 14,5 секунд

Оптимизировано: 10 секунд

Вы заметите, что «оптимизированная» функция быстрее, но более многословна. Его поведение можно изменить, просто изменив константы в его голове.


Извинения, если это кажется немного тривиальным.

Рассмотрим эту функцию.

using System.Text;

public string FizzBanger(int bound)
{
    StringBuilder result = new StringBuilder();
    for (int i = 1; i < bound; i++)
    {
        String line = String.Empty;
        if (i % 3 == 0) line += "fizz";
        if (i % 5 == 0) line += "buzz";
        if (String.IsNullOrEmpty(line)) line = i.ToString();
        result.AppendLine(line.ToString());
    }
    return result.ToString();
}

Вывод будет выглядеть как

1
2
fizz
4
buzz
fizz
7
8
fizz
buzz
11
fizz
13
14
fizzbuzz
16
...

Кто-нибудь может придумать лучший способ сделать это? Пожалуйста, учитывайте как производительность, так и удобство обслуживания.

Ответы [ 3 ]

12 голосов
/ 14 апреля 2011
StringBuilder result = new StringBuilder();

Для фиксированной верхней границы (100) я не стал бы беспокоиться об этом, но хорошо…

StringBuilder line = new StringBuilder();

Но это StringBuilderэто не только избыточно, это действительно неэффективно.И мне даже не нужно тестировать, чтобы знать это.

if (line.Length == 0)

И это просто затеняет логику (это должно реализовать проблему «fizzbuzz», верно?).Сделайте логику явной.

Обратите внимание как на производительность, так и на удобство обслуживания.

Это неправильный способ.Во-первых, ремонтопригодность, во-вторых, производительность (если вообще).Ваш код на самом деле довольно неэффективен, но это не имеет значения: есть 100 итераций - производительность вообще не имеет значения.

Кроме того, какие издержки на сопровождение имеет этот код?Это образец игрушек с фиксированными характеристиками.Нет проблем с ремонтопригодностью.Я бы даже не стал придумывать здесь что-нибудь необычное, Linq решает это автоматически:

return Enumerable.Range(1, bound - 1).Aggregate("",
    (accu, i) =>
        string.Format("{0}\n{1}", accu,
            i % 15 == 0 ? "fizzbuzz" :
            i % 3 == 0 ? "fizz" :
            i % 5 == 0 ? "buzz" : i.ToString()));

Но я согласен, что это может ухудшить читабельность, если не использовать функцию Aggregate.Так что сделайте это более явным:

var result = new StringBuilder();
for (int i = 1; i < bound; i++)
    result.AppendLine(
        i % 15 == 0 ? "fizzbuzz" :
        i % 3 == 0 ? "fizz" :
        i % 5 == 0 ? "buzz" : i.ToString());
return result.ToString();

Все остальное чрезмерно инженерно.

3 голосов
/ 14 апреля 2011

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

{
      StringBuilder result = new StringBuilder();
      for (int i = 1; i < 101; i++)
      {
           var rest3 = i % 3;
           var rest5 = i % 5;

           if (rest3 == 0) result.Append("fizz");
           if (rest5 == 0) result.Append("bang");
           if (rest3 != 0 && rest5 != 0)
               result.Append(i);

           result.Append(System.Environment.NewLine);
      }
}
0 голосов
/ 22 декабря 2014

Что если мы сделаем вещи немного сложнее? 1) не допускается деление или операции по модулю; 2) Цикл должен пропустить все ненужные итерации. Вот ответ:

int n3 = 3;
int n5 = 5;
int i = 3;
while (i <= 100)
{
    Console.Write(i.ToString() + " - ");

    if (i == n3)
    {
        Console.Write("fizz");

        n3 = n3 + 3;
    }

    if (i == n5)
    {
        Console.Write("buzz");

        n5 = n5 + 5;
    }

    Console.WriteLine();

    i = n3 < n5 ? n3 : n5;
}
...