3
\$\begingroup\$

I have a list of objects each with a unique Key and a display name (Label). The list needs to remain sorted by the keys of each object but unfortunately the keys need to be strings rather than numerical and as such 'sorted' here refers to the keys being sorted alphabetically rather than numerically. (Although the keys can be string of numbers - their actual string values don't matter so long as they're alphabetically ordered).

The list should be re-order-able but then the keys need to re-shuffle so that they're once more in alphabetical order.

Currently my implementation allows for the re-ordering of the list and then iterates over all of the objects re-assigning them fresh keys in alphabetical order - something which I feel is a bad idea as it's a lot of effort for a single swap of values.

My object looks like the following

class Item
{
    public string Key { get; set; }
    public string Label { get; set; }

    public Item(string key, string label)
    {
        Key = key;
        Label = label;
    }

    public override string ToString() => Key + ", " + Label;
}

I have added an extension method to handle the re-ordering process

static class Utils
{
    public static List<Item> MoveIndex(this List<Item> myItems, int from, int to)
    {
        Item temp = myItems[from];
        myItems.RemoveAt(from);
        myItems.Insert(to, temp);

        for (int i = 0; i < myItems.Count; i++)
        {
            myItems[i].Key = i.ToString("D2");
        }

        return myItems;
    }
}

And finally I have an example usage of the extension method

static void Main(string[] args)
{
    List<Item> myItems = new List<Item>()
    {
        new Item("0", "Zero"),
        new Item("1", "One"),
        new Item("2", "Two"),
        new Item("3", "Three"),
        new Item("4", "Four"),
        new Item("5", "Five"),
        new Item("6", "Six"),
        new Item("7", "Seven"),
        new Item("8", "Eight"),
        new Item("9", "Nine"),
        new Item("10", "Ten"),
        new Item("11", "Eleven"),
        new Item("12", "Twelve"),
    };

    myItems.MoveIndex(3, 5);

    Console.ReadKey();
}

As the code currently sits, placing a breakpoint on the Console.ReadKey() shows that the Labels are not in their original order but the Keys are - proving that it currently works.


Are there any better ways to manage the Keys when the list is being re-ordered? Perhaps ways which don't require iterating over everything in the list?

Are there any better strings which I could be using for the keys?


I know that as it sits this system has a capacity for only 100 Items before it breaks and that is OK - There will be checking in place to ensure that there are never more than this.

\$\endgroup\$
3
\$\begingroup\$
    public static List<Item> MoveIndex(this List<Item> myItems, int from, int to)

Code to the interface, not the implementation: you might be able to rewrite this as

    public static IList<Item> MoveIndex(this IList<Item> myItems, int from, int to)

and if not you can almost certainly rewrite it as

    public static TList MoveIndex<TList>(this TList myItems, int from, int to)
        where TList : IList<Item>

        for (int i = 0; i < myItems.Count; i++)
        {
            myItems[i].Key = i.ToString("D2");
        }

For a start, this looks too tightly bound to me. From the description given I would expect MoveIndex to work with the existing keys, whatever they are.

But given the strong assumptions made here, I wonder whether there's a better solution based on eliminating the List<Item> entirely. Can you maintain an IList<string> which contains the labels and then generate the items lazily with IEnumerable<string>.Select(Func<string, int, Item>)?

\$\endgroup\$
2
\$\begingroup\$

If your need is max 100 items in the list, I think your implementation is OK.


public static List<Item> MoveIndex(this List<Item> myItems, int from, int to)
{
    Item temp = myItems[from];
    myItems.RemoveAt(from);
    myItems.Insert(to, temp);
    for (int i = 0; i < myItems.Count; i++)
    {
        myItems[i].Key = i.ToString("D2");
    }
    return myItems;
}

Some minor considerations:

If from < to then by removing from, you left shift from from and upwards and maybe invalidate the to index?

To me it's counter intuitive that the list is returned from this method, because you manipulate the input list itself. If returning a list here, I would expect the input list to be untouched and the returned list to be a new instance holding the change.

You can make a minor optimization in that it is only necessary to renumber the entries from Math.Min(from, to) to Math.Max(from, to) (both inclusive) because everything before and after these indices are untouched.


As Peter Taylor writes, it's common good practice to code for interfaces rather than implementations, but be aware that not all IList<s> implementations support RemoveAt(int index) and Insert(int index, T value). For instance arrays don't. So in this special case it's maybe a good idea to stick with the List<Item>

\$\endgroup\$
0
\$\begingroup\$

You're doing a lot of work for no gain. If you really can guarantee that there will never be more than 100 items in the list, why not go ahead and create 100 strings, store them, and assigned them the the items during your ordering phase.

You can just reuse the same set of strings each time, never do any work formatting, and get some efficiency because the ordering step would just be running through a loop assigning values from one array (in order) to the objects in the other array (in order):

for (int i = 0; i < myItems.Count; i++)
{
    myItems[i].Key = listOf100Strings[i];
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.