Is there a better way to return the next item in a list and loop from the end to the front?

Question

I have the following list of distinct strings:

"A"
"B"
"C"

If I want the item after A, I get B. After B, I get C. After C, I get A. Currently I have the following code, but for some reason it feels to me that there is a better way to go about this (maybe?).

private string GetNext(IList<string> items, string curr)
{
    if (String.IsNullOrWhitespace(curr))
        return items[0];

    var index = items.IndexOf(curr);
    if (index == -1)
        return items[0];

    return (index + 1 == items.Count) ? items[0] : items[index + 1];
}

I'm definitely open to a LINQ-esque way of doing this as well :)

I think that this: http://stackoverflow.com/questions/716256/creating-a-circually-linked-list-in-c is what you're looking for — hyp, Apr 23 '12 at 15:41
@hyp: Well, this would work if the parameter being passed in was a `CircularLinkedList`, but it is just an `IList`. — myermian, Apr 23 '12 at 15:47

JaredPar · Answer 1 · 2012-04-23T17:35:12.073

The solution you have is functionally correct but it's performance leaves a little to be desired. Typically when dealing with a list style structure you would expect that GetNext would return a result in O(1) time yet this solution is O(N).

public sealed class WrappingIterator<T> {
  private IList<T> _list;
  private int _index;
  public WrappingIterator<T>(IList<T> list, int index) {
    _list = list;
    _index = index;
  }
  public T GetNext() {
    _index++;
    if (_index == _list.Count) {
      _index = 0;
    }
    return _list[_index];
  }

  public static WrappingIterator<T> CreateAt(IList<T> list, T value) {
    var index = list.IndexOf(value);
    return new WrappingIterator(list, index);
  }
}

The initial call to CreateAt is O(N) here but subsequent calls to GetNext are O(1).

IList<string> list = ...;
var iterator = WrappingIterator<string>.CreateAt(list, "B");
Console.WriteLine(iterator.GetNext());  // Prints C
Console.WriteLine(iterator.GetNext());  // Prints A
Console.WriteLine(iterator.GetNext());  // Prints B

This is a very interesting way of going about it. A bit more complicated than I'd like, but definitely interesting. — myermian, Apr 23 '12 at 15:55
Also, just an FYI. The only reason I did not accept this answer is because the function takes in an `IList`, and I didn't want to create an new iterator for each call to the function. — myermian, Apr 23 '12 at 17:31

score 6 · Accepted Answer · edited Apr 23 '12 at 15:56

6

I think maybe you can change the line

return (index + 1 == items.Count) ? items[0] : items[index + 1];

for something like

return items[(index + 1) % items.Count];

edited Apr 23 '12 at 15:56

myermian

31,823
24
123
215

answered Apr 23 '12 at 15:40

Andres

3,324
6
27
32

1

Good improvement, I'll take that. Though it should be (index + 1) right? – myermian Apr 23 '12 at 15:41
I don't think so, because in your array Count = 3 and when you get Index = 3 it will return 3%3 == 0. When you get index = 2 (to return 'C') you will get it right 2%3 = 2 – Andres Apr 23 '12 at 15:49
Except index is zero-based, so I would never get 3 back for the third item, instead i'd get 2. The first item would return 0. Also, as a bonus I can remove the `if (index == -1)` check because `(-1 + 1) % Anything` is zero always. Yay. – myermian Apr 23 '12 at 15:54
just to mention: this is neither safe against empty lists not against a null reference, but the original implementation isn't, either. – eFloh Apr 23 '12 at 16:28
@eFloh: Yea, I didn't get to the part where I check against a null or empty list, which should throw exceptions. I just haven't added my exception handling (Contracts) yet. – myermian Apr 23 '12 at 17:33

score 1 · Answer 3 · answered Apr 23 '12 at 15:42

I can see some optimization if you track the current index rather than the current string, but to do that the list of items would have to be fixed, i.e. not change.

You could also return items[(index + 1) % items.Count];

Otherwise that code looks fine to me, but perhaps someone has a more clever solution.

score 1 · Answer 4 · edited May 23 '17 at 12:17

1

LINQ is not the appropriate tool here.

It sounds as if a LinkedList<T> would be the better collection here:

var linkedItems = new LinkedList<String>(items);
LinkedListNode current = linkedItems.Find("C");
String afterC = current.Next == null ? linkedItems.First.Value : current.Next.Value;

Here are the pros and cons of a LinkedList compared to a List.

edited May 23 '17 at 12:17

Community

1
1

answered Apr 23 '12 at 15:49

Tim Schmelter

450,073
74
686
939

score 1 · Answer 5 · answered Apr 23 '12 at 15:53

1

A linq way:

var result = (from str in list
              let index = list.IndexOf(curr) + 1
              select list.ElementAtOrDefault(index) ?? list[0]).First();

answered Apr 23 '12 at 15:53

Joe

80,724
18
127
145

score 0 · Answer 6 · answered Apr 23 '12 at 15:42

You can use the mod operator to simplify this a bit and combine everything into one statement:

return items[((String.IsNullOrWhitespace(curr) 
                ? 0 
                : items.IndexOf(curr)) + 1) % items.Count]

Its definitely shorter, but i'm not sure weather its more readable, too :)

score 0 · Answer 7 · answered Apr 23 '12 at 15:52

0

I think the best solution is in the below link, I tried it out and works like charm.

http://www.dotnetperls.com/sort-list

answered Apr 23 '12 at 15:52

Ravi Vanapalli

9,805
3
33
43

Is there a better way to return the next item in a list and loop from the end to the front?

7 Answers7