Looping trought a list of objects in C# in a better way?

Question

I have a foreach loop that loops trought a list of objects. The meaning of it is to set a NavigateUrl to a Hyperlink. My code looks like this:

foreach (var con in contacts)
        {
            if (con.ContactTypeID == 1)
            {
                FacebookIcon.NavigateUrl = "http://facebook.com/" + con.ContactURL;
            }
        }

I wonder if their is some better way to do it. I will have about 10 other ContactTypeID and I rather don't write nine more if else..

I would create method in your **Contact** class and make smth like - `con.getNavigateUrl()` — Aleksej Vasinov, Apr 18 '12 at 08:44
What do you want to do for each ContactTypeID - it would help us understand if you did a couple if/else statements so we got the gist of what you are trying to accomplish. Also, do you expect there to be only one contact with contacttypeid = 1 or could you have 10 contacts with that ID? — Prescott, Apr 18 '12 at 08:45

Tim Schmelter · Accepted Answer · 2012-04-18T09:44:54.527

You could use LINQ:

var facebookURL = contacts.Where(c => c.ContactTypeID == 1)
              .Select(c => c.url)
              .FirstOrDefault();
if(facebookURL != null)
    FacebookIcon.NavigateUrl = "http://facebook.com/" + facebookURL;

Edit: Actually you could benefit of LINQ's deferred execution to reuse the same for every type of contact-type:

var contactType = 1; // facebook
var url = contacts.Where(c => c.ContactTypeID == contactType)
      .Select(c => c.url);
if (url.Any())
    FacebookIcon.NavigateUrl = "http://facebook.com/" + url.First();
contactType = 2;    // google
if (url.Any())
    GoogleIcon.NavigateUrl = "http://Google.com/" + url.First();

Edit 2: Here's another approach using a Dictionary mapping all types with their URLs which should be more efficient in case you have millions of types ;-) (@MAfifi):

var urlTypeMapping = contacts.GroupBy(c => c.ContactTypeID)
    .ToDictionary(grp => grp.Key, grp => grp.Select(c => c.url));
foreach (var type in urlTypeMapping)
{
    var typeUrl = type.Value.FirstOrDefault();
    if (typeUrl != null)
    {
        switch (type.Key)
        {
            case 1:
                FacebookIcon.NavigateUrl = "http://facebook.com/" + typeUrl;
                break;
            case 2:
                GoogleIcon.NavigateUrl = "http://Google.com/" + typeUrl;
                break;
            default:
                break; //or throw new Exception("Invalid type!");
        }
    }
}

I believe this could lead to a NullReferenceException. If there are no contacts with contactTypeID == 1, you'll receive an empty list. In that case, the Select statment will throw. — Frederik Gheysels, Apr 18 '12 at 08:47
Inefficient because you have to enumerate several times over contacts, once for each contact type. — M Afifi, Apr 18 '12 at 08:50
@MAfifi: Added another approach using a Dictionary. Note that i would always prefer the most readable approach when the faster 1.) requires more time to implement 2) will only be faster a few millis and in case of millions of iterations what is unlikely here. — Tim Schmelter, Apr 18 '12 at 09:24
Still maintain my answer is better ;). Simpler to read/maintain, and still more efficient — M Afifi, Apr 18 '12 at 09:29

score 3 · Answer 2 · answered Apr 18 '12 at 08:40

3

You could use LINQ in order to do what you want.

var x = contacts.FirstOrDefault (c => c.ContactTypeID == 1);

if( x != null )
{
   FacebookIcon.NavigateUrl = String.Format ("http://facebook.com/{0}", x.ContactURL);
}

answered Apr 18 '12 at 08:40

Frederik Gheysels

56,135
11
101
154

Inefficient because it means you have to enumerate over contacts 10 times (once for every contact type). – M Afifi Apr 18 '12 at 08:49
@MAfifi: No this is not the point => FirstOrDefault is fine. String.Format("foo{0}", suffix) is an overkill since String.Format internally creates a new StringBuilder and "parses" the pattern etc. "foo" + suffix is definitely better... (and in the end it doesn't matter) – Stefan Apr 18 '12 at 09:03
Insignificant different between concat and String.Format, more on some results here, http://stackoverflow.com/questions/16432/c-sharp-string-output-format-or-concat. Even after a million objects being concated / string formatted, there is a difference of less than 500 msec. – M Afifi Apr 18 '12 at 09:27

score 0 · Answer 3 · answered Apr 18 '12 at 08:40

0

You can use switch

switch (caseSwitch)
{
case 1: 
    FacebookIcon.NavigateUrl = "http://facebook.com/" + con.ContactURL;
    break;
case 2:
    //
    break;
default:
    //
    break;

}

answered Apr 18 '12 at 08:40

Likurg

2,742
17
22

score 0 · Answer 4 · answered Apr 18 '12 at 08:40

0

you can use linq.

var con = contacts.FirsOrDefault(c => c.ContactTypeID.Equals(1));
if (con == null)
{
  return;
}

con.NavigateUrl = "http://facebook.com/" + con.ContactURL;

Or if you have more id's

List<int> ids = new List<int> {1,2,5,7};
contacts.Where(c => ids.Containt(c.ContactTypeID)).ToList().ForEach(item => item.NavigateUrl = "http://facebook.com/" + item.ContactURL);

answered Apr 18 '12 at 08:40

Kamil Lach

4,519
2
19
20

The foreach would set the navigateurl to the last match, this would set it to the first - could be different. – Prescott Apr 18 '12 at 08:43
depends on context you can use FirstOrDefault or LastOrDefault. – Kamil Lach Apr 18 '12 at 08:45

score 0 · Answer 5 · answered Apr 18 '12 at 08:41

0

If you use Linq You should use First or FirstOrDefault

var url = contacts.FirstOrDefault(c => c.ContactTypeID == 1).NavigateUrl;

answered Apr 18 '12 at 08:41

Preben Huybrechts

5,853
2
27
63

score 0 · Answer 6 · answered Apr 18 '12 at 08:42

Use a switch:

foreach (var con in contacts) 
    { 
        switch (con.ContactTypeID) 
        {
          case 1: 
            FacebookIcon.NavigateUrl = "http://facebook.com/" + con.ContactURL; 
            break;
          case 2:
            . . . 
            break;
          . . . 
        } 
    }

score 0 · Answer 7 · answered Apr 18 '12 at 08:45

0

Perhapse a condensed LINQ:

contacts.ForEach(c => { if (c.ContactTypeID  == 1) FacebookIcon.NavigateUrl = "http://facebook.com/" + con.ContactURL;  });

If you want to do "it" for each ContactTypeID == 1.

answered Apr 18 '12 at 08:45

kaze

4,299
12
53
74

score 0 · Answer 8 · answered Apr 18 '12 at 08:45

I assume that each contact is not necessarily Facebook and you need to dynamically set different attributes based on what the contact is?

Your best bet is a Dictionary<int, Action> or similar, where you just do something like,

var setCorrectUrl = new Dictionary<int, Action<Contact>>
{
    // Appropriate entries in here, e.g. (syntax not quite right)
    {
        1,
        (contact) => FacebookIcon.NavigateUrl = contact.ContactURL;
    }
}

foreach (var con in contacts)
{
    setCorrectUrl[con.ContactTypeID](con);
}

score 0 · Answer 9 · answered Apr 18 '12 at 09:07

Well, I would do some refactoring of the code. Imagine he would have 10 other types to implement :) The solutions provided above are workable but not very elegant in terms of extensibility.

So, here is my solution:

1) Implement a base class with the common contact properties

public abstract class BaseContact
{
    public string Name { get; set; }
    public abstract string Url { get; set; }

}

2) Implement the concrete types

public class FbContact : BaseContact
{
    private string _baseUrl = "http://facebook.com/{0}";
    private string _url = string.Empty;

    public override string Url
    {
        get { return _url; }
        set { _url = string.Format(_baseUrl, value); }
    }
}

public class LinkedInContact : BaseContact
{
    private string _baseUrl = "http://linkedin.com/{0}";
    private string _url = string.Empty;

    public override string Url
    {
        get { return _url; }
        set { _url = string.Format(_baseUrl, value); }
    }
}

3) This is just a helper class for setting the navigation url

public static class NavigationCreator
{
    public static void SetUrl(BaseContact contact, HyperLink link)
    {
        link.NavigateUrl = contact.Url;
    }
}

4) Some test code to visualize the result

        List<BaseContact> items = new List<BaseContact>();

        for (int i = 0; i < 5; i++)
        {
            BaseContact item;
            if (i % 2 == 0) item = new FbContact(); else item = new LinkedInContact();

            item.Url = "My name " + i;

            items.Add(item);
        }

        foreach (var contact in items)
        {
            HyperLink link = new HyperLink();
            NavigationCreator.SetUrl(contact, link);
            Console.WriteLine(link.NavigateUrl);
        }

        Console.Read();

Looping trought a list of objects in C# in a better way?

9 Answers9