Terminal.Gui icon indicating copy to clipboard operation
Terminal.Gui copied to clipboard

ListView ListWrapper - marking does work if you give an IList which in which the count changes

Open BoldInventions opened this issue 2 years ago • 2 comments

The symptoms are that if I use listView.SetSource with an IList with an initial count of zero, when I later add items to the list, listView.SetMark() does do anthing at all.

To reproduce it, you can simply try giving listView.SetSource an IList with zero count, then add some lines to it. Then try marking items.

Here's why I think it is happening:

ListView.SetSource takes an IList object. I need to dynamically add/remove things from the list, so I wrote my own IList object. However, the BitArray 'marks' is constructed once when the ListWrapper is constructed, and thus has a fixed size. Since my IList object is of zero count at startup, SetMark just doesn't work at all.

BoldInventions avatar Oct 11 '23 11:10 BoldInventions

This definetly sounds like a bug or something that could be improved.

Are you using v1 or v2 of Terminal.Gui? One solution might be to implement IListDataSource directly?

tznind avatar Oct 11 '23 12:10 tznind

I'm using v1 of Terminal.Gui, but the current source I believe has the same problem.

I made my own ListWrapper class which copied the code, but took out the member variable 'count', and made both SetMark() and IsMarked() check the size of the BitArray and if the array is smaller than Count, it makes a new BitArray, copies the smaller to the new one and then replaces it. Seems to work great!

BTW - I love Terminal.Gui


void checkAndResizeMarksIfRequired()
{
   if(Count > marks.Length)
   {
      int i;
      BitArray newMarks = new BitArray(Count);
      for(i=0; i<marks.Length; i++) newMarks[i] = marks[i];
      marks = newMarks;
   }
}

/// <inheritdoc/>
public bool IsMarked (int item)
{
   checkAndResizeMarksIfRequired();
   if (item >= 0 && item < Count) return marks [item];
     return false;
}

/// <inheritdoc/>
public void SetMark (int item, bool value)
{
   checkAndResizeMarksIfRequired();
   if (item >= 0 && item < Count)
   marks [item] = value;
}

BoldInventions avatar Oct 11 '23 12:10 BoldInventions

List<T> does not contain any events for changes in the collection or anything else and the only solution, using the current IList implementation, is to check if there is any change in the collection when the Source.Count property is accessed. That's why it's very limited. Another better solution is to use another class such as ObservableCollection<T> that has the CollectionChanged event, allowing the ListWrapper to be notified when there are changes to the collection.

BDisp avatar May 25 '24 21:05 BDisp

Side note about this for V2:

IList is an awful interface from the days of .net Framework 1.0 and is typically not recommended to be used in modern projects, if it can be avoided.

Instead, IEnumerable<T>, IList<T>, or others like them are recommended, as IList is both non-generic (boxing alert!) and is not a trustworthy interface.

What do I mean by not trustworthy?

Well, it has Add and other methods you'd expect on a List. Cool, right?

Not cool.

Because System.Array implements IList (note the "Implements" list), and a call to Add on an IList that is actually not a List, such as an Array, will always throw a NotSupportedException, which is one of the major blunders of .net Framework 1.0 that 2.0, with generics and such, fixed, but only in new interfaces such as IList<T>, since it's blasphemous to make a breaking change to an existing interface (which is good, but man that's an unfortunate historical caveat).

V2 should never use IList. If it does (I have not checked), we should fix that.

dodexahedron avatar May 26 '24 05:05 dodexahedron

I also don't know why it's difficult for List<T> to raise a CollectionChanged event on the Count property because just changing that property is enough to notify the client.

BDisp avatar May 26 '24 09:05 BDisp

It just doesn't implement that interface.

ObservableCollection<T> does, but I wouldn't recommend using that for most situations.

dodexahedron avatar May 26 '24 10:05 dodexahedron

I know :-) I just mentioned that the dotnet team could implement it in List<T> so as not to force the use of ObservableCollection<T>, because this one, for example, does not implement AddRange and GetRange, although it is easy to implement them in a derived class. But for the user, it is much easier to use types that already exist in .NET than to use derived classes.

BDisp avatar May 26 '24 11:05 BDisp

I know :-) I just mentioned that the dotnet team could implement it in List<T> so as not to force the use of ObservableCollection<T>, because this one, for example, does not implement AddRange and GetRange, although it is easy to implement them in a derived class. But for the user, it is much easier to use types that already exist in .NET than to use derived classes.

I wish for that, too. :(

I hate having to do it myself for other collections.

There are nuget packages out there with some, but I usually only need one or maybe the packages that have the one I need do something strange that I don't want.

*sigh*

C# and dotnet are great, except when they aren't. 😅

dodexahedron avatar May 29 '24 00:05 dodexahedron