spectreconsole / spectre.console

A .NET library that makes it easier to create beautiful console applications.
https://spectreconsole.net
MIT License
9.22k stars 476 forks source link

TableRowCollection inherits IReadOnlyList, which does not provide IndexOf #1596

Open GNUGradyn opened 1 month ago

GNUGradyn commented 1 month ago

Is your feature request related to a problem? Please describe. When working with tables, you often want to remove a specific element from the table. Assuming your code does not work out what the index of every element should be before it inserts it and keeps track of all of them (which it obviously shouldn't have to do) there is no way to figure out the index of the element you want to remove. Since the only exposed method to remove an element is based on its index, this is a problem.

IReadOnlyList, unlike IReadOnlyCollection, does expect indexing and therefore should have an IndexOf method. However, rather it be due to weird complicated historical reasons or due to Microsoft simply overlooking this, it does not (discussion on stack overflow). Microsoft has been aware of this for quite some time and it is still not fixed. Probably because it would either require introducing a massive breaking change to the API or a limited extension method. Regardless, doesn't look like this will be properly fixed in .NET any time soon so the spectre.console API will need to provide a solution.

Describe the solution you'd like There are a few ways this could be approached but perhaps the simplest would be to just add a .IndexOf method to TableRowCollection itself.

Describe alternatives you've considered Another good option would be to add a .Remove() function to TableRowCollection that does not use the index at all. However if this is implemented it should probably be in addition to the above fix because there are other reasons users might want to know the index of the row.


Please upvote :+1: this issue if you are interested in it.

GNUGradyn commented 1 month ago

I just discovered it might not even be possible to implement this without modifying TableRow since it doesn't implement IComparable and also seems to lose its reference somewhere along the line so CompareReference won't work either. You can't even extend TableRow to contain an identifier because its sealed. As it stands there is no way, not by extending TableRow nor contain an identifier nor by adding an extension method to the collection, to match up a TableRow in the collection. This should probably be a high priority because it means the only way to use this library is to track the indexes of everything you insert into the table yourself