Найти номера подряд и превращать их в диапазоны


У меня есть массив

int[] arr = { -2, 0, 1, 2, 3, 4, 5, 8, 9, 11, 13, 15, 18, 22, 25, 28, 29, 30 };

Мне нужно написать функцию, которая показывает его как строку, и если цифры рядом, например, 1,2,3,4,5 - я хочу показать им 1-5.

Я написал эту функцию в репозиторий

public string[] GetArray()
{
    int[] arr = { -2, 0, 1, 2, 3, 4, 5, 8, 9, 11, 13, 15, 18, 22, 25, 28, 29, 30 };

    int? lastValue = arr.FirstOrDefault();

    var groupDataIntoAdjacentBlocks = arr.Segment(i =>
    {
        var result = lastValue != null && (lastValue != (i - 1));
        lastValue = i;
        return result;
    }
    );

    var convertSetsIntoRanges = groupDataIntoAdjacentBlocks.Select(z =>
    {
        var data = z.ToList();
        if (data.Count() == 1)
            return data.First().ToString();

        return data.First().ToString() + "-" + data.Last().ToString();
    });

    var finalresult = (string.Join(",", convertSetsIntoRanges));
    return new[] { finalresult };

}

И назвать его, как это в контроллер

public JsonResult GetArray()
{
    var arrayresult = _arrayrepo.GetArray();
    return Json(arrayresult, JsonRequestBehavior.AllowGet);
}

Любые предложения, как я могу улучшить мой код?



696
2
задан 12 февраля 2018 в 09:02 Источник Поделиться
Комментарии
4 ответа

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


Первое продолжение знает только как найти диапазоны. Если у вас нет класса/структуры, то вы можете использовать кортежи.

public static IEnumerable<(int Min, int Max)> ToRanges(this IEnumerable<int> source)
{
using (var e = source.GetEnumerator())
{
if (!e.MoveNext())
{
yield break;
}

var previous = e.Current;

var range = (Min: previous, Max: e.Current);

while (e.MoveNext())
{
var isConsecutive = e.Current - previous == 1;
if (isConsecutive)
{
range = (range.Min, e.Current);
}
else
{
yield return range;
range = (e.Current, e.Current);
}
previous = e.Current;
}

yield return range;
}
}

Другое расширение потребляет результат ToRanges и возвращает строку.

public static string Stringify(this IEnumerable<(int Min, int Max)> ranges)
{
var values = ranges.Select(range => $"{(IsEmpty(range) ? $"{range.Min}" : $"{range.Min}-{range.Max}")}");
return string.Join(", ", values);

bool IsEmpty((int Min, int Max) range)
{
return range.Max - range.Min == 0;
}
}


Преимущества наличие двух разделенных методы:
- вы можете сосредоточиться на одной задаче за один раз
- теперь вы можете проверить только один объект за раз
- вы можете повторно использовать ToRange для расчета их длины при необходимости


Другие вещи, которые вам следует обратить внимание:
- имена переменных не должны звучать, как они были методами, например, groupDataIntoAdjacentBlocks; гораздо лучше будет имя adjacentBlocks или что-то вдоль тех же линий
- int? lastValue = arr.FirstOrDefault(); это неправильно, поскольку значение по умолчанию для int это 0 и не null.
- результат GetArray() способ не имеет особого смысла; зачем вам возвращать массив строк, если есть всегда только один элемент? Я думаю, это не ваш реальный код, но в дальномерной части и редактировал его ради вопрос, поэтому я не буду комментировать это.

5
ответ дан 13 февраля 2018 в 06:02 Источник Поделиться

Это был просто быстрый ответ... но я, кажется, написал большую стену текста: сожалею об этом.

GetArray API-интерфейс

Мне интересно, есть ли веская причина, что это метод экземпляра; скорее, я предполагаю, что это очень конкретную реализацию GetArray способ на какой-то интерфейс. Я говорю это, потому что без контекста это выглядит как ужасный интерфейс API! Вы странно назвали упомянутого метода (что делает GetArray в смысле?) не используя любой инстанции государственной, и которая имеет очень необычный тип возврата: все это имеет смысл, однако, если это реализация интерфейса, который специально предназначен для создания JSON, но я все равно думаю, можно придумать имя получше!

Даже если это очень специфический интерфейс реализации, я бы не прочь разорвать большую часть этого в (статические) вспомогательная функция, поскольку в настоящее время у вас есть все сложность выполнения задач фаршированные в метод, который действительно заботится только о результате, а не как там у вас, и что какая-то странная возвращаемое значение, которое не имеет ничего общего с группировкой логики. Я бы сильно склонны нарушать Из 2 методов, один для выполнения группировки и другие для получения строк из группы, только потому, что чувствует, как "обработка данных", а другой чувствует себя как "форматирование данных" и это разные идеи, работа с различными типами (как производить строку и засунул ее в массив в пользу JSON имеют разные идеи). Вы можете перейти с любой степенью детализации хочешь, но я бы, конечно, хотите, чтобы вытащить код finalResult из:

/// <summary>
/// Produces a string representation of a sequence of integers, combining contiguous values into a single range
/// For example: {1, 2, 3, 5} -> "1-3,5"
/// </summary>
public static string ProduceContiguousIntegersString(IEnumerable<int> integers)
{
if (integers == null)
throw new ArgumentException("The list of integers may not be null", nameof(integers));

// snip
}

Примечание параметр типа: вы не только выполняя вперед проходит через массив, так что нет никаких причин, чтобы ограничить потребителю проходит мимо тебя массив: IEnumerable<T> Это самый простой тип вам здесь в качестве входных данных. Как всегда, некоторые эксплуатационной документации (\\\) всегда ценится на любом публичном API. Я бросил null регистрация там так же просто как пример: общественного API должны всегда проверять входные и кинуть значимые исключения, если это используется внутренне, то это менее важно, но все равно полезно.

Это, как вы можете назвать этот новый метод:

public string[] GetArray()
{
int[] arr = { -2, 0, 1, 2, 3, 4, 5, 8, 9, 11, 13, 15, 18, 22, 25, 28, 29, 30 };

var finalResult = ProduceContiguousIntegersString(arr);
return new[] { finalResult };
}

Как RadarBob сказал, с arr испеченный в этот метод просто выглядит неправильно. Я взял на себя смелость изменить "finalresult" в "finalResult", потому что это два слова, но я уверен, что вы могли бы придумать более meaningul имя, если ты дал ему еще немного подумать.

lastValue

lastValue странно. Вы, кажется, сделали его значение null, так что вы можете справиться с пустым массивом (который кричит "Арр не должна быть жестко"), но это все немного туманно, и не понятно что на выходе будет, если ты сдашь в пустой массив. Я был бы склонен предоставить явную "массив пустой" чек.

Лучше использовать возможность, хотел сделать его явным, что это не значимое значение, но: действительно, можно заменить arr.FirstOrDefault(); С null и ваш код будет работать, и будет менее запутанной даже! На самом деле... вы можете снять флажок в целом, потому что код будет не работать , если массив пуст! Любые такие проверки (например, что это не null) чрезмерно оборонительных, и будет работать только непонятных глюков в будущем.

'Реальный' проблема с этот код, что Segment имеет немного странный интерфейс API. Я это Segment это отличная идея, но это-из-ужасные ожидая сегментация функция отслеживать собственное состояние, и это не ясно, будет ли он называет это лямда на первый элемент или нет. Предполагая, что мы не можем связываться с Segmentмой инстинкт должен пойти с что-то вроде этого:

public static string ProduceContiguousIntegersString(IEnumerable<int> integers)
{
// handle special case of empty enumerable
if (!integers.Any())
{
return "";
}

int previousValue = integers.First();
var groupDataIntoAdjacentBlocks = integers.Segment(i =>
{
bool newSegment = previousValue != (i - 1);
previousValue = i;
return newSegment;
}
);

// ...

Чтобы быть действительно придирчивым, я бы предпочел, чтобы его называли "previousValue", потому что "функция lastvalue" может означать последнее значение в массиве.

z.ToList()

Что z? Я могу простить i в первый цикл, но что z? И почему ты превращаешь ее в List<T>? В худшем , вы должны быть превратив его в массив, так как вам не нужен динамический тип данных, и вместо того, чтобы назначать на ВАР, вы могли бы назначить IReadOnlyList<int>, который гарантирует, что вы не можете случайно изменить что-либо. В любом случае, вы не только выполнение простых операций по z (который я переименовал в block ниже), поэтому там не так много вреда, если вы оставите его в качестве IEnumerable<T> или что она уже есть. (Если вы беспокоитесь о стоимости Count()тогда идите с T[] как IReadOnlyList подход).

    // ...

var convertSetsIntoRanges = groupDataIntoAdjacentBlocks.Select(block =>
{
if (block.Count() == 1)
return segment.First().ToString();

return block.First().ToString() + "-" + block.Last().ToString();
});

var finalResult = (string.Join(",", convertSetsIntoRanges));
return finalResult;
}

Я бы также пересмотреть имени переменной groupDataIntoAdjacentBlocks если бы я был тобой: для меня это читается так, как будто он производит какие-то блоки, которые находятся рядом, но на самом деле вы производите некоторые блоки, которые содержат соседних элементов.

Сортировка??

Как RadarBob предполагает, выглядит, как будто этот код ожидает отсортированный массив. Возможно, вам нужно отсортировать массив перед обработкой его? Если вы беспокоитесь о производительности, вы можете сделать первый пас, чтобы проверить, если массив уже отсортирован, и только его, если необходимо; в противном случае, просто документ, в котором четко параметров.

2
ответ дан 12 февраля 2018 в 11:02 Источник Поделиться

Я бы, наверное, внести следующие изменения:


  1. Переименовать способ получить что-то более описательное, как GetRangeString.

  2. Сделать метод static. Так как нет необходимости в другое государство, это облегчает для клиента.

  3. Есть метод возвращает строку, так что это действительно все, что он создает.

  4. Есть способ принимать массив целых чисел в качестве входных данных. Как она стоит сейчас, поскольку ваш метод создает массив внутренне, он может также просто вернуть жестко-закодированных строк.

  5. Построить строку диапазона на один проход по массиву. Код может выглядеть не так хорошо, но я считаю, что это гораздо более эффективным, чем создание сегменты, выбрать те сегменты, в списки и строительства колеблется от тех

  6. Доступ к элементам по индексу (как arr[0]), а не с помощью методов расширения, как arr.First() если это возможно. Если вы посмотрите на исходный код IEnumerable.First()вы заметите, что он выполняет проверку на null, делает IList бросок, еще один нулевой чек, подсчитать, проверить, и, наконец, возвращает элемент в index[0]. Это был быстрее всего это сделать в первую очередь.

Я не знаю, что Segment метод. Это метод расширения, который вы создали? Я хотел бы сделать тест производительности между вашим кодом и ниже код, но не можете использовать ваш существующий код.

Вот пример что я имею в виду:

public static string GetRangeString(int[] arr)
{
// Return fast if array is null or contains less than 2 items
if (arr == null || !arr.Any()) return string.Empty;
if (arr.Length == 1) return arr[0].ToString();

var rangeString = new StringBuilder();
bool isRange = false;

for (int i = 0; i < arr.Length; i++)
{
while (i < arr.Length - 1 && arr[i] + 1 == arr[i + 1])
{
if (!isRange) rangeString.Append($"{arr[i]}");
isRange = true;
i++;
}

if (isRange)
{
rangeString.Append("-");
isRange = false;
}

rangeString.Append($"{arr[i]},");
}

return rangeString.ToString().TrimEnd(',');
}

Использование

static void Main()
{
var arr = new[] { 1, 5, 2, 3, 4, 6, 8, 9, 10, 11 };
Console.WriteLine(GetRangeString(arr));

Console.Write("\nPress any key to exit...");
Console.ReadKey();
}

Выход

enter image description here

2
ответ дан 13 февраля 2018 в 04:02 Источник Поделиться

Сдать в массиве. В противном случае этот код просто не полезно.


public string[] GetArray(int[] someIntegers)


Сделать массив из диапазона строк, не разделенных запятыми значений строки. Иначе нет никакого смысла в возвращении int[].


Отсортировать исходный массив.


Работает этот код? Жесткого кодирования массива в "успех оптимизирован" орден скрывает ошибки.


Массив подходит для ваших нужд? Интересно о конвертировании ToList() прямо в середине обработки. Ваши данные должны быть в рабочем структуры с самого начала, и остальная часть кода не должны беспокоиться об этом.

        var data = z.ToList();
if (data.Count() == 1)
return data.First().ToString();

1
ответ дан 12 февраля 2018 в 10:02 Источник Поделиться