7
\$\begingroup\$

I'm developing a Tic Tac Toe game where you can change your pawn look

enter image description here

Now as You can see it has 2 buttons one for next one for previous, and You can also pick if you want to edit the O or the X sprite. Now here comes the custom collection I made and the reason for that is because I need something that can remember what was the last element shown/accessed. It's really similar to how IEnumerator works, but the problem is that IEnumerator cant move backwards, which is some functionality I need. It's my first time creating my own collection, so any flaws or ways to improve it are appreciated (and maybe a better name for it..). I also added the AddRange() function and also a converter ToIterable().

public class IterableList<T> : IList<T>
{
    private readonly IList<T> _iterableList = new List<T>();

    private int lastUsedElementIndex;

    public T MoveNext
    {
        get
        {
            int temp = lastUsedElementIndex;
            lastUsedElementIndex = lastUsedElementIndex + 1 >= _iterableList.Count ? 0 : lastUsedElementIndex + 1;      
            return _iterableList[temp];
        }
    }

    public T MovePrevious
    {
        get
        {
            int temp = lastUsedElementIndex;
            lastUsedElementIndex = lastUsedElementIndex - 1 < 0 ? _iterableList.Count - 1 : lastUsedElementIndex - 1;
            return _iterableList[temp];
        }
    }

    public T Current
    {
        get
        {
            if(lastUsedElementIndex + 1 >= _iterableList.Count)
            {
                return _iterableList[0];
            }
            return _iterableList[lastUsedElementIndex + 1];
        }
    }

    public IterableList(int StartingIterableIndex = 0)
    {
        lastUsedElementIndex = StartingIterableIndex;
    }

    public IterableList<T> AddRange(IList collection, int startingIterableIndex)
    {
        IterableList<T> iterableCollection = new IterableList<T>(startingIterableIndex);
        foreach (var item in collection)
        {
            iterableCollection.Add((T)item);
        }
        return iterableCollection;
    }

    public IterableList<T> AddRange(IList<T> collection, int startingIterableIndexn)
    {
        IterableList<T> iterableCollection = new IterableList<T>(startingIterableIndexn);
        foreach (var item in collection)
        {
            iterableCollection.Add(item);
        }
        return iterableCollection;
    }

    public T this[int index]
    {
        get { return _iterableList[index]; }
        set { _iterableList[index] = value; }
    }

    public IEnumerator<T> GetEnumerator()
    {
        return _iterableList.GetEnumerator();
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }

    public int Count
    {
        get { return _iterableList.Count; }
    }

    public bool IsReadOnly
    {
        get { return _iterableList.IsReadOnly; }
    }

    public void Add(T item)
    {
        _iterableList.Add(item);
    }

    public void Clear()
    {
        _iterableList.Clear();
    }

    public bool Contains(T item)
    {
        return _iterableList.Contains(item);
    }

    public void CopyTo(T[] array, int arrayIndex)
    {
        _iterableList.CopyTo(array, arrayIndex);
    }

    public int IndexOf(T item)
    {
        return _iterableList.IndexOf(item);
    }

    public void Insert(int index, T item)
    {
        _iterableList.Insert(index, item);
    }

    public bool Remove(T item)
    {
        return _iterableList.Remove(item);
    }

    public void RemoveAt(int index)
    {
        _iterableList.RemoveAt(index);
    }
}

And the extension converter method

     public static IterableList<T> ToIterable<T>(this IList collection, int startingIterableIndex)
    {
        IterableList<T> iterableCollection = new IterableList<T>(startingIterableIndex);
        return iterableCollection.AddRange(collection, startingIterableIndex);
    }

    public static IterableList<T> ToIterable<T>(this IList<T> collection, int startingIterableIndex)
    {
        IterableList<T> iterableCollection = new IterableList<T>(startingIterableIndex);
        return iterableCollection.AddRange(collection, startingIterableIndex);
    }

One last note if you wonder why the constructor takes a parameter int StartingIterableIndex = 0 it's because when you press O/X button it instantly shows you the first item of the collection and IterableList will also start from the first item thus showing the same sprite twice, it's really a specific case and that's why it's optional parameter not a required one.

\$\endgroup\$

3 Answers 3

5
\$\begingroup\$
  • The getter of the properties MoveNext and MovePrevious change the internal state of the class. That is a really uncommon behavior. I would suggest you change the properties to methods.
  • The method AddRange creates a new list, but doesn't use the items of the list where AddRange was called. I would rename it to Create and make it static or remove that method and provide an appropriated constructor instead.

Actually, your use case does not require to sublcass a list. You need something that works on a list. Lets call it CircularListNavigator:

public class CircularListNavigator<TItem>
{
    private readonly IList<TItem> _internalList;
    public ListNavigator(IList<TItem> list)
    {
        _internalList = list;
    }

    public TItem Current { get { ... } }
    public void MoveNext() { ... }
    public void MovePreviouse() { ... }
} 
\$\endgroup\$
2
\$\begingroup\$

You can do it much easier because what you actually need is a way to navigate through the list back and forth. This can be solved with a simple ListNavigator:

public class ListNavigator<T>
{
    private readonly IList<T> _list;

    public ListNavigator(IList<T> list) { _list = list; }

    public int Index { get; private set; }

    public T Current => _list[Index];

    public T MoveNext()
    {
        ++Index;
        if (Index >= _list.Count) { Index = 0; }
        return _list[Index];
    }

    public T MovePrev()
    {
        --Index;
        if (Index < 0) { Index = _list.Count - 1; }
        return _list[Index];
    }
}

Let the list take care of adding, removing, inserting etc. and do the navigation with another specialized structure.

\$\endgroup\$
1
  • \$\begingroup\$ I know... you hate alternative solutions... but maybe someone else will find it more useful ;-P \$\endgroup\$
    – t3chb0t
    Aug 8, 2016 at 20:33
1
\$\begingroup\$

If anyone is interested in the final version, I happen to need the same collection again and added a few improvements while taking in consideration the suggestions from the answer's posted here

public class CircularListNavigator<T> : IList<T>
{
    private readonly IList<T> _circularListNavigator = new List<T>();

    private int lastUsedElementIndex;

    public T MoveNext()
    {
        int temp = lastUsedElementIndex;
        lastUsedElementIndex = lastUsedElementIndex + 1 >= _circularListNavigator.Count ? 0 : lastUsedElementIndex + 1;
        return _circularListNavigator[temp];
    }

    public T MovePrevious()
    {
        int temp = lastUsedElementIndex;
        lastUsedElementIndex = lastUsedElementIndex - 1 < 0 ? _circularListNavigator.Count - 1 : lastUsedElementIndex - 1;
        return _circularListNavigator[temp];
    }

    public T Current
    {
        get
        {
            return _circularListNavigator.Count == 0 ? default(T) : _circularListNavigator[lastUsedElementIndex];
        }
    }

    public void Reset()
    {
        lastUsedElementIndex = 0;
    }

    public CircularListNavigator(int StartingIterableIndex = 0)
    {
        lastUsedElementIndex = StartingIterableIndex;
    }

    public CircularListNavigator(IEnumerable<T> source)
    {
        _circularListNavigator = source.ToCircularListNavigator();
    }

    public CircularListNavigator<T> ConvertToCircularListNavigator(IEnumerable<T> collection, int startingIterableIndex)
    {
        CircularListNavigator<T> iterableCollection = new CircularListNavigator<T>(startingIterableIndex);
        foreach (var item in collection)
        {
            iterableCollection.Add(item);
        }
        return iterableCollection;
    }

    public T this[int index]
    {
        get { return _circularListNavigator[index]; }
        set { _circularListNavigator[index] = value; }
    }

    public IEnumerator<T> GetEnumerator()
    {
        return _circularListNavigator.GetEnumerator();
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }

    public int Count
    {
        get { return _circularListNavigator.Count; }
    }

    public bool IsReadOnly
    {
        get { return _circularListNavigator.IsReadOnly; }
    }

    public void Add(T item)
    {
        _circularListNavigator.Add(item);
    }

    public void Clear()
    {
        _circularListNavigator.Clear();
    }

    public bool Contains(T item)
    {
        return _circularListNavigator.Contains(item);
    }

    public void CopyTo(T[] array, int arrayIndex)
    {
        _circularListNavigator.CopyTo(array, arrayIndex);
    }

    public int IndexOf(T item)
    {
        return _circularListNavigator.IndexOf(item);
    }

    public void Insert(int index, T item)
    {
        _circularListNavigator.Insert(index, item);
    }

    public bool Remove(T item)
    {
        return _circularListNavigator.Remove(item);
    }

    public void RemoveAt(int index)
    {
        _circularListNavigator.RemoveAt(index);
    }
}

And the extensions

public static class Extensions
{     
    public static CircularListNavigator<T> ToCircularListNavigator<T>(this IEnumerable<T> collection)
    {
        CircularListNavigator<T> circularListNavigator = new CircularListNavigator<T>();
        foreach (object item in collection)
        {
            circularListNavigator.Add((T)item);
        }
        return circularListNavigator;
    }
}
\$\endgroup\$

This site is temporarily in read-only mode and not accepting new answers.

Not the answer you're looking for? Browse other questions tagged .