RelayCommand stops working after a while

Question

I am facing some problems using GalaSoft's RelayCommand.

I have a NextCommand property that works, but only several times.

Afterwards, it stops working completely.

You can try this out with the sample project:

http://s000.tinyupload.com/?file_id=65828891881629261404

The behaviour is as follows:

1. NextCommand:
   1. pops all items until the active index
   2. if there are less than 50 items left, pushes 1 new item
   3. marks new item as active
2. BackCommand:
   1. moves the active index back by 1 position

Steps to replicate:

1. the '+' (OemPlus) key has been bound to NextCommand
2. the '-' (OemMinus) key has been bound to BackCommand
3. Hold the '+' key until the list stops growing (50 items limit)
4. Hold the '-' key until the first item in the list is the active
5. Repeat

The number of repetitions needed (to replicate the bug) is inconsistent.

Sometimes I get it after 4 repetitions; other times up till 9.

// Items Collection
public class ItemCollection : ViewModelBase
{
    // List of Items
    private readonly ObservableCollection<Item> _items = new ObservableCollection<Item>();
    public ObservableCollection<Item> Items
    {
        get { return _items; }
    }

    // Constructor
    public ItemCollection()
    {
        BackCommand = new RelayCommand(
                () =>
                {
                    // Go to previous page
                    var index = Items.IndexOf(ActiveItem);
                    if (index > 0)
                    {
                        ActiveItem = Items[index - 1];
                    }
                },
                () => ActiveItem != null && Items.IndexOf(ActiveItem) > 0);
    }

    // Back command
    public RelayCommand BackCommand { get; set; }

    // Next command
    public RelayCommand NextCommand { get; set; }

    // The currently-active item
    private Item _activeItem;
    public Item ActiveItem
    {
        get { return _activeItem; }
        set
        {
            Set(() => ActiveItem, ref _activeItem, value);
        }
    }
}

// Item
public class Item : ViewModelBase
{
    public string Title { get; set; }
}

When I stepped into the RelayCommand's code, the execute action's isAlive flag was false. But I can't seem to figure out how that might happen.

Acidosis

+1 for your nice animated Gif explaining your issue. – Creaturely

I don't see where is NextCommand instantiated in your code. The problem could be there (something what disable it). – Surly

In other words, we need to see what NextCommand is – Dumuzi

Your sample project is not available anymore – Schoolgirl

Answer 1

Two words: Garbage Collector

In your example project--which you should post the relevant bits of to make your question future-proof--you set the DataContext on your window like this:

public partial class MainWindow : Window
{
    public MainWindow()
    {
        InitializeComponent();

        var logic  = new LogicObject();
        DataContext = logic.Collection;
    }
}

Because nothing else retains a reference to the LogicObject created here, it will be collected at the next opportunity.

The command stops functioning because in LogicObject, you set the NextCommand of the ItemCollection to use private members of the soon-to-be-collected LogicObject:

public class LogicObject
{
    public LogicObject()
    {
        Collection = new ItemCollection();
        Collection.NextCommand = new RelayCommand(AddItem, CanAddItem);
        AddItem();
    }

    private bool CanAddItem()
    {
        // snip...
    }

    private void AddItem()
    {
        // snip...
    }
}

Once LogicObject is collected, the command can no longer work because it no longer has references to valid methods (AddItem and CanAddItem). This is why the isAlive field on both of the RelayCommand's weak references to the methods is false.

You can fix this by just hanging on to the LogicObject, or by moving the AddItem and CanAddItem methods into the collection.

---

To get in the spirit of GIFs for this question, here's one that shows the button stop working as soon as a Gen 0 collection occurs.

Vasques

I see it now. Always knew that RelayCommand uses WeakActions and WeakReferences, but still managed to miss the part where my LogicObject wasn't referenced anywhere else. Thanks for the answer! (and great gif btw) – Acidosis

As of MVVMLight 5.4.0 you can use the keepTargetAlive constructor parameter to keep closures from being garbage-collected: mvvmlight.net/doc/weakaction.cshtml – Infantilism

Answer 2

why dont you simply use the methods from ICollectionView? there you have:

• MoveCurrentTo
• MoveCurrentToFirst
• MoveCurrentToLast
• MoveCurrentToNext
• MoveCurrentToPrevious
• and other nice stuff

something like this

 private ICollectionView MyView {get;set;}


 this.MyView = CollectionViewSource.GetDefaultView(this._items);


 if (!this.MyView.IsCurrentBeforeFirst)
 {
     this.MyView.MoveCurrentToPrevious();
 }

Saffier

hi blindmeis. I wasn't really looking for collection operations. I just happened to name the sample classes ItemCollection and Item. The key part of the code is the NextCommand property, that is configurable by its caller. – Acidosis