Refactor foreach and if's to Linq

Question

Would it be possible to combine all the given code to linq, which then would return 'true' or 'false'? Or is this fine the way it is?

If it is possible, would there be a significant performance difference?(The array and list won't contain more than 100 elements)

foreach (var item in myArray)
{
    if (myList.Exists(x => x.Value == item))
    {
        amountTrue++;
    }
}

if (myArray.Count() == amountTrue)
{
    isValid = true;
}

Don't refactor for performance but readability and maintainability. — Tim Schmelter, May 12 '16 at 11:17
If you just need to know if all elements are in the list. Simply check in the loop if one element is not in the list then break. — Yann39, May 12 '16 at 11:22

score 11 · Accepted Answer · answered May 12 '16 at 11:23

11

As far as I can see, the code is designed to check

if all the items of the myArray are in the myExists

the implementation:

  isValid = myArray
    .All(item => myList.Exists(x => x.Value == item));

if you want to refactor for performace as well as for readability, have a look for bottlenecks. Say, is it myList.Exists that slows down? In that case think of HashSet<T> where T is a type of x.Value etc.

answered May 12 '16 at 11:23

Dmitry Bychenko

180,369
20
160
215

You have to be careful with LINQ's All() method. It's a vacuous truth statement. As a result isValid would end up being true if myArray were empty. Best to check to test if myArray is empty in addition to All(). #BattleScars – Rafael Dowling Goodman May 12 '16 at 11:34
@Rafael Dowling Goodman: as I can see from your current code, if the array is empty then `amountTrue == 0` and thus `(myArray.Count() == amountTrue)` is `true`; my suggested refactoring does the same – Dmitry Bychenko May 12 '16 at 11:38

score 7 · Answer 2 · edited May 12 '16 at 11:56

7

If I understand the problem, you want to know if all items in the array exist in the list:

bool isValid = !myArray.Except(myList.Select (l => l.Value )).Any();

Then this would be an efficient approach because Except uses a set and Any stops on the first missing.

edited May 12 '16 at 11:56

Tim Schmelter

450,073
74
686
939

answered May 12 '16 at 11:27

Stefan

111
3

@Dmitry Bychenko Solved it very efficient and easy to read. With your code you make it harder to understand what you are doing. – Oscar Vicente Perez May 12 '16 at 11:29
This is indeed the most efficient approach if only the `bool` result is needed. +1 (added an explanation) – Tim Schmelter May 12 '16 at 11:53
@OscarVicentePerez Perhaps harder for the the untrained eye but more efficient. +1 – Magnus May 12 '16 at 12:17
Searching about Except impl I found this: http://stackoverflow.com/questions/10269610/how-does-except-method-work-in-linq. So I have to upvote for the efficiency part (but not for the readability) – Oscar Vicente Perez May 12 '16 at 12:24

Tim Schmelter · Answer 3 · 2016-05-12T11:35:37.053

5

Don't refactor for performance but readability and maintainability. But yes, there is a LINQ query which should be efficient and readable:

bool isValid = myArray.All(item => myList.Any(x => x.Value == item));

If you also need to know the count you have multiple options:

for example with Enumerable.Any:

int amountTrue = myArray.Count(item => myList.Any(x => x.Value == item));

or more efficient with a HashSet<T>, presuming string:

var uniqueValues = new HashSet<string>(myList.Select(x => x.Value));
int amountTrue = myArray.Count(uniqueValues.Contains);

edited May 12 '16 at 11:35

answered May 12 '16 at 11:21

Tim Schmelter

450,073
74
686
939

1

You are doing too much work, see the @Dmitry Bychenko answer – Oscar Vicente Perez May 12 '16 at 11:25
@OscarVicentePerez: noticed it later, now i've provided multiple ways – Tim Schmelter May 12 '16 at 11:34
I know, i just tell it to Tim. Now I find it's answer way better. – Oscar Vicente Perez May 12 '16 at 11:37

score 3 · Answer 4 · answered May 12 '16 at 11:26

3

Here is the LINQ implementation

var res = ((from z in myArray
            where myList.Exists(x=>x.Value=z)
            select z).Count())==myArray.Count()

answered May 12 '16 at 11:26

Dmitri Soshnikov

727
4
7

6

@OscarVicentePerez the existence of a (in your opinion) slightly better answer is not a very good reason to down vote other good and correct answers. – René Vogt May 12 '16 at 11:30
The reason of the down vote wasn't the existing answer. It's the readability and performance, as he is doing more than need. – Oscar Vicente Perez May 12 '16 at 11:41

Refactor foreach and if's to Linq

4 Answers4