Проблема с циклической оптимизацией или закрытием лямбды? - PullRequest
10 голосов
/ 15 июня 2011

В следующем методе я отправляю перечисление действий и хочу получить обратно массив ICommands, который вызывает Action<object>, который оборачивает эти действия (необходимые для relayCommand).

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

    public static ICommand[] CreateCommands(IEnumerable<Action> actions)
    {
        List<ICommand> commands = new List<ICommand>();

        Action[] actionArray = actions.ToArray();

        // works
        //commands.Add(new RelayCommand(o => { actionArray[0](); }));  // (_execute = {Method = {Void <CreateCommands>b__0(System.Object)}})
        //commands.Add(new RelayCommand(o => { actionArray[1](); }));  // (_execute = {Method = {Void <CreateCommands>b__1(System.Object)}})

        foreach (var action in actionArray)
        {
            // always add the same _execute member for each RelayCommand (_execute = {Method = {Void <CreateCommands>b__0(System.Object)}})
            commands.Add(new RelayCommand(o => { action(); }));
        }

        return commands.ToArray();
    }

Кажется, что лямбда всегда повторно используется внутри цикла, думая, что делаетто же самое, но это не так.

Как мне преодолеть эту ситуацию?Как я могу заставить цикл к угрозе o => { action(); } всегда как новый?

Спасибо!

Что я пробовал согласно предложениям, но не помогло:

        foreach (var action in actionArray)
        {
            Action<object> executeHandler = o => { action(); };
            commands.Add(new RelayCommand(executeHandler));
        }

Мне кажется, что работает:

    class RelayExecuteWrapper
    {
        Action _action;

        public RelayExecuteWrapper(Action action)
        {
            _action = action;
        }

        public void Execute(object o) 
        {
            _action();
        }
    }

/// ...
    foreach (var action in actionArray)
    {
        RelayExecuteWrapper rxw = new RelayExecuteWrapper(action);
        commands.Add(new RelayCommand(rxw.Execute));
    }

Код RelayCommand:

/// <summary>
/// A command whose sole purpose is to 
/// relay its functionality to other
/// objects by invoking delegates. The
/// default return value for the CanExecute
/// method is 'true'.
/// </summary>
public class RelayCommand : ICommand
{
    #region Fields

    readonly Action<object> _execute;
    readonly Predicate<object> _canExecute;        

    #endregion // Fields

    #region Constructors

    /// <summary>
    /// Creates a new command that can always execute.
    /// </summary>
    /// <param name="execute">The execution logic.</param>
    public RelayCommand(Action<object> execute)
        : this(execute, null)
    {
    }

    /// <summary>
    /// Creates a new command.
    /// </summary>
    /// <param name="execute">The execution logic.</param>
    /// <param name="canExecute">The execution status logic.</param>
    public RelayCommand(Action<object> execute, Predicate<object> canExecute)
    {
        if (execute == null)
            throw new ArgumentNullException("execute");

        _execute = execute;
        _canExecute = canExecute;           
    }

    #endregion // Constructors

    #region ICommand Members

    [DebuggerStepThrough]
    public bool CanExecute(object parameter)
    {
        return _canExecute == null ? true : _canExecute(parameter);
    }

    public event EventHandler CanExecuteChanged
    {
        add { CommandManager.RequerySuggested += value; }
        remove { CommandManager.RequerySuggested -= value; }
    }

    public void Execute(object parameter)
    {
        _execute(parameter);
    }

    #endregion // ICommand Members
}

Ответы [ 4 ]

27 голосов
/ 15 июня 2011

Эта проблема сообщается несколько раз в неделю в StackOverflow.Проблема заключается в том, что каждая новая лямбда, созданная внутри цикла, имеет общую переменную , аналогичную .Лямбды не фиксируют значение, они захватывают переменную.То есть, когда вы говорите

List<Action> list = new List<Action>();
foreach(int x in Range(0, 10))
    list.Add( ()=>{Console.WriteLine(x);} );
list[0]();

, это, конечно, печатает «10», потому что это значение x сейчас .Действие - «записать текущее значение x», а не «записать значение, которое x вернуло при создании делегата».

Чтобы обойти эту проблему, создайте новую переменную:

List<Action> list = new List<Action>();
foreach(int x in Range(0, 10))
{
    int y = x;
    list.Add( ()=>{Console.WriteLine(y);} );
}
list[0]();

Поскольку эта проблема настолько распространена, мы рассматриваем возможность изменения следующей версии C #, чтобы новая переменная создавалась каждый раз в цикле foreach.

См. http://ericlippert.com/2009/11/12/closing-over-the-loop-variable-considered-harmful-part-one/ для получения дополнительной информации.

ОБНОВЛЕНИЕ: Из комментариев:

Каждая ICommand имеет один и тот же метод информации:

{ Method = {Void <CreateCommands>b__0(System.Object)}}

Да, конечно, это так.Метод один и тот же каждый раз.Я думаю, вы неправильно понимаете, что такое создание делегата.Рассмотрим этот вариант.Предположим, вы сказали:

var firstList = new List<Func<int>>() 
{ 
  ()=>10, ()=>20 
};

ОК, у нас есть список функций, которые возвращают целые числа.Первый возвращает 10, второй возвращает 20.

Это то же самое, что и:

static int ReturnTen() { return 10; }
static int ReturnTwenty() { return 20; }
...
var firstList = new List<Func<int>>() 
{ ReturnTen, ReturnTwenty };

Имеет смысл пока?Теперь мы добавим ваш цикл foreach:

var secondList = new List<Func<int>>();
foreach(var func in firstList)
    secondList.Add(()=>func());

ОК, что означает , что означает?Это означает то же самое, что и:

class Closure
{
    public Func<int> func;
    public int DoTheThing() { return this.func(); }
}
...
var secondList = new List<Func<int>>();
Closure closure = new Closure();
foreach(var func in firstList)
{
    closure.func = func;
    secondList.Add(closure.DoTheThing);
}

Теперь понятно, что здесь происходит?Каждый раз в цикле вы не создаете новое замыкание и, конечно, не создаете новый метод.Создаваемый вами делегат всегда указывает на один и тот же метод и всегда на одно и то же замыкание.

Теперь, если вместо этого вы написали

foreach(var loopFunc in firstList)
{
    var func = loopFunc;
    secondList.Add(func);
}

, то сгенерированный нами код будет

foreach(var loopFunc in firstList)
{
    var closure = new Closure();
    closure.func = loopFunc;
    secondList.Add(closure.DoTheThing);
}

Теперь у каждой новой функции в списке есть такой же метод информации - это все еще DoTheThing - но другое закрытие .

Теперь имеетимеет смысл, почему вы видите свой результат?

Возможно, вы захотите также прочитать:

Каково время жизни делегата, созданного лямбда-выражением в C #?

ДРУГОЕ ОБНОВЛЕНИЕ: Из отредактированного вопроса:

То, что я пытался согласно предложениям, но не помогло:

    foreach (var action in actionArray)         
    {
         Action<object> executeHandler = o => { action(); };
         commands.Add(new RelayCommand(executeHandler));         } 
    }

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

Правильный и короткий способ решения проблемы -

    foreach (var action in actionArray)         
    {
         Action<object> copy = action;
         commands.Add(new RelayCommand(x=>{copy();}));
    }

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

Каждый делегат будет иметь methodinfo но другое закрытие .

Я не совсем уверен насчет этих закрытий и лямбд

Вы делаете выше-Закажите функциональное программирование в вашей программе. Тебе лучше узнать об "этих замыканиях и лямбдах", если ты хочешь иметь возможность делать это правильно. Нет времени, подобного настоящему.

1 голос
/ 15 июня 2011

Я только что сделал рабочий пример того, что вы пытаетесь сделать: http://ideone.com/hNcGx

    interface ICommand
    {
        void Print();
    }

    class CommandA : ICommand
    {
        public void Print() { Console.WriteLine("A"); }
    }

    class CommandB : ICommand
    {
        public void Print() { Console.WriteLine("B"); }
    }

    public static void Main()
    {
        var actions = new List<Action>();
        foreach (var command in new ICommand[]{
                    new CommandA(), new CommandB(), new CommandB()})
        {
            var commandcopy = command;
            actions.Add(() => commandcopy.Print());
        }

        foreach (var action in actions)
            action();
    }

Вывод:

A
B
B

Помогает ли это?

0 голосов
/ 16 июня 2011

Вы когда-либо использовали только первый элемент в массиве actionArray.

е

commands.Add(new RelayCommand(o => { actionArray[0](); }));

Вам нужно перебрать коллекцию действий.

например

public static ICommand[] CreateCommands(IEnumerable<Action> actions)
{
  commands = actions.Select(o => new RelayCommand(o)).ToArray();
}

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

0 голосов
/ 15 июня 2011

Сделать локальную ссылку на action в области видимости цикла.

foreach (var action in actionArray)
{ 
   var myAction = action;
   // always add the same _execute member for each RelayCommand (_execute = {Method = {Void <CreateCommands>b__0(System.Object)}})
   commands.Add(new RelayCommand(o => { action(); }));
}
...