views:

136

answers:

1

Hi,

I'm having alot of trouble with my code. When I compile I get the following error:


'Ecommerce.DataHelpers.ProductNodeLoader' does not implement interface member 'System.Collections.IEnumerable.GetEnumerator()'. 'Ecommerce.DataHelpers.ProductNodeLoader.GetEnumerator()' cannot implement 'System.Collections.IEnumerable.GetEnumerator()' because it does not have the matching return type of 'System.Collections.IEnumerator'.


Im not sure how to solve this so now I have to ask you guys!

CODE:

namespace Ecommerce.DataHelpers
{
    public class ProductNodeLoader<T> : IEnumerable<T>
    {
        private ISqlHelper sqlHelper;
        private IRecordsReader nodeReader;

        public List<T> list = new List<T>();

        // load all products from given company
        public IEnumerator<T> GetEnumerator()
        {
            int companyId = 2;
            try
            {
                sqlHelper = DataLayerHelper.CreateSqlHelper(GlobalSettings.DbDSN);
                nodeReader = sqlHelper.ExecuteReader(@"
                    SELECT * FROM eCommerceNodes WHERE companyId = @companyId)
                    ", sqlHelper.CreateParameter("@companyId", companyId));

            }
            catch (Exception e)
            {
                Log.Add(LogTypes.Custom, -1, e.InnerException.ToString());
                yield break;
            }

            if (nodeReader.HasRecords)
            {
                while(nodeReader.Read())
                {
                    ProductNode node = new ProductNode();
                    node.id = nodeReader.Get<int>("id");
                    node.parentId = nodeReader.Get<int>("parentId");
                    node.companyId = nodeReader.Get<int>("companyId");
                    node.path = nodeReader.Get<string>("path");
                    node.sortOrder = nodeReader.Get<string>("sortOrder");
                    node.text = nodeReader.Get<string>("text");
                    node.nodeType = nodeReader.Get<int>("nodeType");

                    list.Add(node);

                }
                nodeReader.Close();

            }
            else
            {
                throw new ApplicationException("No products to load");
            }

            return list;
        }

    }
}

I apologize for the bad editing!

+7  A: 

You're trying to implement the non-generic IEnumerable type too, as IEnumerable<T> extends it. Fortunately, this is easy:

// Along with the existing using directives
using System.Collections;
...

// In the implementing class
IEnumerator IEnumerable.GetEnumerator()
{
    return GetEnumerator();
}

Note that you have to use explicit interface implementation for at least one of the two GetEnumerator() methods you need to implement, as they have the same signature (same name, no parameters) but different return types. Fortunately, the return value of the generic version is fine to use for the non-generic version, which is why the above pattern is usually used.

EDIT: As Josh correctly notes in the comments, you have other problems too:

  • You shouldn't have a return list; at the end of your code, unless you remove the yield break; earlier (and change it to return list.GetEnumerator();). If you want to keep your code as an iterator block, you should be using yield return to yield each node you create.
  • You should be yielding instances of T - whereas you're constructing instances of ProductNode. Perhaps you should actually be implementing IEnumerable<ProductNode> instead of IEnumerable<T>, and make your class non-generic?
  • Your code will keep the SQL connection open for as long as the caller decides to take iterating over it. That may or may not be a problem - but it's worth bearing in mind.
  • You should use a using statement to make sure that your nodeReader is disposed on error (assuming the caller disposes of the IEnumerator<T> of course)
  • Your public list field is a bad idea... why have you made it an instance variable at all?
Jon Skeet
I dare not try to compete here, but it is worth noting that this is not his *only* problem. There is a return statement inside the method which is not allowed, and he is returning a concrete instance of ProductNode where a generic type T is expected. In general he should be doing a **yield return** instead of adding the item to a list.
Josh
@Josh: Edited to add those points and a few others.
Jon Skeet
Thank you guys for all your help!My first version did only contain "yield return node". But after trying everything I could think of i got kinda desperate =)A new question that has come up is this on: Why cant a yield command be used within try clause?
Marthin
@Marthin: It can. It can't be used within a finally clause though. See this blog series for lots of design details: http://blogs.msdn.com/b/ericlippert/archive/tags/iterators/
Jon Skeet
Great Help! Thx!
Marthin