Preferred style

[Reiss]

I have written the following

List UserProjects = ...

string domain = cbxDomain.SelectedValue.ToString();
HashSet itemsToShow = new HashSet(); // See annotation 0

And I was wondering which of the two snippets below people prefer A) Use a Predicate

foreach (Project p in UserProjects.FindAll(delegate(Project x){return x.DomainName.Equals(domain);}))
{
itemsToShow.Add(p.ProjectName);
}

Or B) Nested if

foreach (Project p in UserProjects)
{
if (p.DomainName.Equals(domain))
{
itemsToShow.Add(p.ProjectName);
}
}

0If you haven't used HashSet before, it is basically a List that sort of behaves like a Dictionary in that doesn't allow duplicates, but doesn't throw an exception if you try and add a duplicate. (I know that's an over simplification, - see MSDN[^] for full description)

[Lost User]

Given the choices, the second. FindAll with an old-style anonymous function is a bit ugly in my opinion. Do you have the option of replacing that with a LINQ query?

[Paul Conrad]

I personally would go with the second choice for the sake of readability.

"The clue train passed his station without stopping." - John Simmons / outlaw programmer "Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon "Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham

[PIEBALDconsult]

The latter. But why are you usnig .Equals ?

[Luc Pattyn]

I'll join the crowd and prefer the latter for its readability; the former isn't bringing anything IMO. :)

Luc Pattyn [My Articles] Nil Volentibus Arduum

[SledgeHammer01]

why not just use: if (!itemsToShow.Contains(p.ProjectName)) itemsToShow.Add(p.ProjectName); gets rid of the foreach and the ugly LINQ statement. Please stop using LINQ for every little thing... the performance hit you take is big :). People who use LINQ all over there code often find themselves re-writting slow chunks down the road.

[Lost User]

While the second part is true,

SledgeHammer01 wrote:

why not just use: [something]

Because it does something entirely different.

[SledgeHammer01]

Er, you're right... I just glossed over the example :).

[BobJanova]

Option A should really be

foreach (Project p in UserProjects.FindAll(x => x.DomainName.Equals(domain)))
{
itemsToShow.Add(p.ProjectName);
}

But I still think B is clearer. Also, why are you using .Equals instead of ==? For almost every class they are equivalent (even if I override Equals for value semantics I also override ==/!= because it's just too confusing otherwise) and == is prettier.

[Reiss]

Thanks for your comments - this was more a general style point rather than the actual semantics of the sample code - I am putting together some new coding standards for a client and wanted to garner your views.