List‹IEnumerator›.All(e =› e.MoveNext()) не перемещает мои перечислители дальше

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

class Program
{
    static void Main(string[] args)
    {
        var ints = new List<List<int>> {
            new List<int> {0, 0, 1},    // This row has a 1 at index 2
            new List<int> {0, 1, 0},    // This row has a 1 at index 1
            new List<int> {0, 0, 1}     // This row also has a 1 at index 2
        };
        var result = IndexesWhereThereIsOneInTheColumn(ints);

        Console.WriteLine(string.Join(", ", result)); // Expected: "1, 2"
        Console.ReadKey();
    }


    private static IEnumerable<int> IndexesWhereThereIsOneInTheColumn(
        IEnumerable<List<int>> myIntsGrid)
    {
        var enumerators = myIntsGrid.Select(c => c.GetEnumerator()).ToList();

        short i = 0;
        while (enumerators.All(e => e.MoveNext())) {
            if (enumerators.Any(e => e.Current == 1))
                yield return i;
            i++;

            if (i > 1000)
                throw new Exception("You have gone too far!!!");
        }
    }

}

Однако я заметил, что MoveNext() не запоминается каждый раз в цикле while. MoveNext() всегда возвращает true, а Current всегда 0. Является ли это целенаправленной функцией Linq, чтобы сделать его более свободным от побочных эффектов?

Я заметил, что это работает:

    private static IEnumerable<int> IndexesWhereThereIsOneInTheColumn(
        IEnumerable<List<int>> myIntsGrid)
    {
        var enumerators = myIntsGrid.Select(c => 
            c.ToArray().GetEnumerator()).ToList(); // added ToArray() 

        short i = 0;
        while (enumerators.All(e => e.MoveNext())) {
            if (enumerators.Any(e => (int)e.Current == 1)) // added cast to int
                yield return i;
            i++;
        }
    }

Так это просто проблема со списком?


person Jonny    schedule 08.05.2014    source источник
comment
Почему бы вам просто не использовать циклы foreach?   -  person Rik    schedule 08.05.2014
comment
@Rik Я не смог найти красивую реализацию с использованием foreach. В моей первой попытке было два break, и я думаю, что реализация Linq чище.   -  person Jonny    schedule 08.05.2014
comment
Функции, передаваемые в LINQ, не должны иметь побочных эффектов.   -  person CodesInChaos    schedule 08.05.2014
comment
Сводит с ума! Я только что провел час в этой конкретной проблеме, пытаясь убедить себя, что я не сумасшедший...   -  person josh2112    schedule 20.12.2017


Ответы (6)


Как говорит Sriram Sakthivel, проблема возникает из-за отсутствия упаковки и случайной реализации перечислителя списка как struct, а не ссылочного типа. Как правило, от перечислителя не следует ожидать поведения типа значения, так как большинство из них либо предоставляются интерфейсами IEnumerator/IEnumerator<T>, либо сами являются ссылочными типами. Быстрый способ обойти это - изменить эту строку

var enumerators = myIntsGrid.Select(c => c.GetEnumerator()).ToList();

to

var enumerators 
    = myIntsGrid.Select(c => (IEnumerator) c.GetEnumerator()).ToList();

вместо.

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


Если вам нужен универсальный перечислитель (чтобы избежать приведения при последнем использовании свойства enumerator.Current), вы можете привести к соответствующему универсальному интерфейсу IEnumerator<T>:

c => (IEnumerator<int>) c.GetEnumerator()

или даже лучше

c => c.GetEnumerator() as IEnumerator<int>

Говорят, что ключевое слово as работает намного лучше, чем прямое приведение типов, и в случае цикла оно может дать существенный выигрыш в производительности. Просто будьте осторожны, что as возвращает null, если приведение не выполняется Согласно Просьба Флатера из комментариев:. В случае OP гарантируется, что перечислитель реализует IEnumerator<int>, поэтому можно безопасно перейти к приведению as.

person Ivaylo Slavov    schedule 08.05.2014
comment
Придирка: используйте (IEnumerator<int>) вместо (IEnumerator), чтобы вам не приходилось приводить e.Current внутри цикла while. - person sloth; 08.05.2014
comment
@DominicKexel, хороший момент, я включу его в качестве редактирования. Спасибо - person Ivaylo Slavov; 08.05.2014
comment
Это не из-за бокса. Код ОП изменяет копию, но не коробочную копию. - person CodesInChaos; 08.05.2014
comment
@CodesInChaos, правильно, выражение boxing используется неправильно. На самом деле проблема заключается в отсутствии упаковки, которая позволяет копировать тип значения. Я отредактировал это, спасибо! - person Ivaylo Slavov; 08.05.2014
comment
@IvayloSlavov: может быть важно отметить, что foo as Bar возвращает null, когда приведение не удалось, тогда как (Bar) foo выдает исключение. Они оба полезны по-своему, зависит от того, как вы хотите справиться с ошибочным приведением :) - person Flater; 08.05.2014
comment
@Flater, да, это важно знать. У меня было это в предыдущей версии поста, но я удалил его, потому что посчитал, что слишком многословен. На самом деле, код вопроса использует List<int>, который гарантированно выдает ненулевое IEnumerator<int>, поэтому ключевое слово as можно безопасно использовать. Все равно спасибо, что указали! - person Ivaylo Slavov; 09.05.2014
comment
@IvayloSlavov: Да, я полагал, что в данном случае это не имеет значения. Но из вашего поста кто-то может сделать вывод, что foo as Bar всегда лучше. Это для тех, кого я упомянул :) - person Flater; 09.05.2014
comment
@Flater, я снова добавил это как примечание для разработчиков. - person Ivaylo Slavov; 09.05.2014

Это потому, что перечислитель List<T> является struct, тогда как перечислитель Array является class.

Поэтому, когда вы вызываете Enumerable.All со структурой, создается копия перечислителя и передается в качестве параметра в Func, поскольку структуры копируются по значению. Таким образом, e.MoveNext вызывается на копии, а не на оригинале.

Попробуй это:

Console.WriteLine(new List<int>().GetEnumerator().GetType().IsValueType);
Console.WriteLine(new int[]{}.GetEnumerator().GetType().IsValueType);

Он печатает:

True
False
person Sriram Sakthivel    schedule 08.05.2014
comment
Это не из-за бокса. Код ОП изменяет копию, но не коробочную копию. - person CodesInChaos; 08.05.2014
comment
@CodesInChaos Хороший улов, пересмотрел мой ответ. Посмотрим, есть ли в этом смысл :) - person Sriram Sakthivel; 08.05.2014
comment
Это была моя первая идея, когда я прочитал вопрос, но я не мог представить, что перечислители иногда действительно реализуются как тип значения. Довольно подвержен ошибкам. - person Mene; 08.05.2014
comment
@Mene: Представьте себе это; Это верно! Он подвержен ошибкам, но помните, что в 99,9999% случаев перечислитель виден только коду, сгенерированному для цикла foreach; создание собственных незапакованных счетчиков — редкий сценарий. Команда дизайнеров считала, что выигрыш в производительности за счет избегания давления на сборы стоил очень редких случаев, подобных этому, когда код сбивает с толку. - person Eric Lippert; 14.05.2014

В качестве альтернативы вы можете сделать это с расширением лямбда

var ids = Enumerable.Range(0,ints.Max (row => row.Count)).
      Where(col => ints.Any(row => (row.Count>col)? row[col] == (1) : false));

or

var ids = Enumerable.Range(0,ints.Max (row=> row.Count)).
      Where(col => ints.Any (row => row.ElementAtOrDefault(col) == 1));
person Tarec    schedule 08.05.2014

Вот простая реализация с использованием циклов и yield:

private static IEnumerable<int> IndexesWhereThereIsOneInTheColumn(
    IEnumerable<List<int>> myIntsGrid)
{
    for (int i=0; myIntsGrid.Max(l=>l.Count) > i;i++)
    {
        foreach(var row in myIntsGrid)
        {
            if (row.Count > i && row[i] == 1)
            {
                yield return i;
                break;
            }
        }
    }       
}

В качестве альтернативы используйте это внутри цикла for:

if (myIntsGrid.Any(row => row.Count > i && row[i] == 1)) yield return i;
person Rik    schedule 08.05.2014
comment
Это возвращает индекс строк, содержащих 1, мне нужны столбцы. - person Jonny; 08.05.2014
comment
Ага, я неправильно понял. Отредактировано. - person Rik; 08.05.2014

Просто для удовольствия, вот аккуратный запрос LINQ, который не вызовет трудно отслеживаемых побочных эффектов в вашем коде:

IEnumerable<int> IndexesWhereThereIsOneInTheColumn(IEnumerable<IEnumerable<int>> myIntsGrid)
{
    return myIntsGrid
        // Collapse the rows into a single row of the maximum value of all rows
        .Aggregate((acc, x) => acc.Zip(x, Math.Max))
        // Enumerate the row
        .Select((Value,Index) => new { Value, Index })
        .Where(x => x.Value == 1)
        .Select(x => x.Index);
}
person AlexFoxGill    schedule 09.05.2014
comment
Решает опубликованную (урезанную) проблему, но на самом деле я не использую целые числа в сетке для реальной проблемы. Однако я мог бы создать агрегатор, который выводит нужное мне значение вверх. - person Jonny; 09.05.2014
comment
Конечно, вам просто нужно изменить Math.Max на функцию, которая выбирает выбранное вами значение, а затем, очевидно, и оператор Where - person AlexFoxGill; 09.05.2014

Почему вы не можете просто получить эти индексы следующим образом:

var result = ints.Select (i => i.IndexOf(1)).Distinct().OrderBy(i => i);

Вроде бы все проще...

person st4hoo    schedule 08.05.2014
comment
Это будет работать для этого примера, но я хотел уменьшить количество кода, который я опубликовал. Моя настоящая ошибка более сложная, но код, который я разместил, демонстрирует суть проблемы. - person Jonny; 08.05.2014