4
\$\begingroup\$

this.Items is a list of products. Each product has a category (product.Attributes["category"]).

I want to render a drop down list, where all the products are grouped by category. This works well, but how could I improve it? Could there be a way to not use a dictionary?

protected override void RenderContents(System.Web.UI.HtmlTextWriter writer)
{
 var groups = new Dictionary<string, List<ListItem>>();
 // sort by category
 foreach (ListItem product in this.Items)
 {
 if (!groups.ContainsKey(category))
 {
 groups.Add(category, new List<ListItem>());
 }
 groups[category].Add(product);
 }
 // render each category
 foreach (var group in groups)
 {
 // <optgroup>
 RenderOptionGroupBeginTag(group.Key, writer);
 // each option 
 group.Value.ForEach(p => this.RenderListItem(p, writer));
 // </optgroup>
 RenderOptionGroupEndTag(writer);
 }
}
Malachi
29k11 gold badges86 silver badges188 bronze badges
asked Sep 30, 2014 at 13:00
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

Since ListItemCollection implements IEnumerable, but not IEnumerable<ListItem>, you have to use Cast before you can use other LINQ methods.

After you do that, you can use GroupBy, as Steve Michael suggested and then work with the result almost exactly like with your groups:

var categoryGroups = this.Items.Cast<ListItem>()
 .GroupBy(i => i.Attributes["category"]);
foreach (var categoryGroup in categoryGroups)
{
 RenderOptionGroupBeginTag(group.Key, writer);
 foreach (var item in group)
 {
 this.RenderListItem(item, writer);
 }
 RenderOptionGroupEndTag(writer);
}

Some other changes I made to the code:

  • I used foreach instead of ForEach(). ForEach() looks like a LINQ method, but it behaves very differently, since it doesn't do any querying. I think that in cases like this, foreach in the most suitable option.
  • Changing groups (and group) to categoryGroups means that the code is clearer and so you can get rid of the loop comment.
  • I also removed the other comments, because I think the code is clear about what it does. If you want to emphasize that RenderOptionGroupBeginTag creates <optgroup>, maybe you should rename it? Probably to something like RenderOptgroupBeginTag.
answered Sep 30, 2014 at 15:58
\$\endgroup\$
0
2
\$\begingroup\$

You could do something like this:

var groupedList = this.Items.GroupBy(i => i.Attributes["category"])
 .Select(g =>new { Name = g.Key, Items = g.ToList()})
 .ToList();

Is that along the lines of what you're looking for?

answered Sep 30, 2014 at 13:20
\$\endgroup\$
6
  • \$\begingroup\$ It could be but this won't compile, System.Web.UI.WebControls.ListItemCollection does not contain a definition for GroupBy... \$\endgroup\$ Commented Sep 30, 2014 at 13:38
  • 1
    \$\begingroup\$ You probably need to add 'using System.Linq;' at the top of your class. ListItemCollection implements IList and ICollection so it should work. But I'm not near Visual Studio right now. \$\endgroup\$ Commented Sep 30, 2014 at 13:50
  • \$\begingroup\$ Yeah it's probably just because of the using statement. Also if you use ReSharper it makes it very clear what is missing and allows you to add the reference easily. Check it out! Makes life easier :) \$\endgroup\$ Commented Sep 30, 2014 at 14:30
  • \$\begingroup\$ I added using System.Linq; and I already use Resharper (which is indeed awesome :) ) but compilation still fails... \$\endgroup\$ Commented Sep 30, 2014 at 15:31
  • 1
    \$\begingroup\$ @Jedidja ListItemCollection implements only the non-generic versions of those interfaces, so it won't work with LINQ like this. \$\endgroup\$ Commented Sep 30, 2014 at 15:41

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.