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\$
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() { ... }
} 
| improve this answer | |
\$\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.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I know... you hate alternative solutions... but maybe someone else will find it more useful ;-P \$\endgroup\$ – t3chb0t Aug 8 '16 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;
    }
}
| improve this answer | |
\$\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.