1

I currently a list of a Supplier class, within that supplier class is a list of orders.

Each order has a userID and an empty string variable for username.

I then have a list of users which contains userID and username.

The way I am doing this now is: 

foreach(supplier s in SupplierList) { foreach (order o in s.childorders) { user u = _users.First(p => p.userid == o.userid); o.username = u.username; } }

I feel this might be a little inefficient and I was wondering if it is possible to compact it down into one linq query?

The logic should be

set supplierslist.childorders.username to the value in _users where supplierslist.childorders.userid == _users.userid.

Im fairly new to Linq so any advice for this would be apreciated, or also if its a bad idea and to leave it as it is / reasons why would be good too.

Thanks

Improve this question

6 Answers 6

Reset to default
2

What you want to do here is iterate over a collection (many collections, really, but it doesn't make a difference) and mutate its members. LINQ is not really targeted at performing mutating operations but rather at querying. You can do it with LINQ, but it's against the spirit of the tool.

If you are constructing the SupplierList yourself, it might be possible to fetch the data appropriately with LINQ so that it comes pre-populated as you want it to be.

Otherwise, I 'd leave the foreach as it is. You can make a dictionary that maps ids to users to make the inner loop faster, but that's your call and it depends on your data size.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

1 
var orderUserPairs = SupplierList .SelectMany(s => s.ChildOrders) .Join(_users, o => o.UserId, u => u.userId, (Order, User) => new {Order, User}); foreach (var orderUserPair in orderUserPairs) orderUserPair.Order.username = orderUserPair.User.username;

Though having both username and userId as part of order looks suspicious.

Improve this answer

1 Comment

Order actually doesnt contain Username, in this case the 'real' order class just contains the userID, I am using MVVM and have a class in between Order and my UI which supports propertychanged and this contains UserName as username is only displayed on the UI.
1

First a question...

It looks like you are operating on every order. Why do you need to cycle through the supplierlist first since you don't seem to be using it inside the loop? Unless there are orders that don't belong to any supplierlist, you might be able to skip that step.

If that isn't the case, then I think you can use a join. If you aren't familiar with the syntax for joins in linq, this is one (simplified) way to approach it:

var x = from S in SupplierList join C in childorders on C.supplierlistID equals S.ID where [whatever you need here if anything] select new { field1, field2}; foreach var y in x { }

Note I assumed a foreign key in childorders to supplierlist. If that isn't the case you will have to modify accordingly.

Hope that helps.

Improve this answer

Comments

1

You need to use SelectMany or join depending on weather you are using linq-to-sql or linq with local collections. If you are using local collections the better way is to use join, else use SelectMany.

Like this...join:

var selection = (from s in SupplierList join o in s.childholders on s.userid equals o.userid select new { username = o.username);

or, in case of linq-to-sql:

var selection = (from s in SupplierList from o in s.childholders select { username = o.username);

You can then use the anonymous type you projected the way you want.

Improve this answer

1 Comment

minor point...i think you need to say 'equals' in your join, not '=='.
1

I agree with Jon, but you could say:

var orders = (from s in supplier from o in s.childorders select new { Order = o, User = _users.First(p => p.userid == o.userid) }).ToList(); foreach(var order in orders) { order.Order.username = order.User.username; }

Untested of course :)

Improve this answer

2 Comments

You're setting username of anonymous type in the last line. You haven't tested it, but our mind-compilation skills are strong!
Oh yeah, gah I've just realised what I've done. The other answers are better than mine anyway. UPDATE: fixed?
1

If users list contains many elements, it can be really slow so I'd use a temporary dictionary:

var userById = users.GroupBy(x => x.userid) .ToDictionary(x => x.Key, x => x.First()); foreach(var order in supplier.SelectMany(x => x.childorders)) { order.username = userById[order.userid].username; }
Improve this answer

9 Comments

It was full of typos, fixed (I hope) :S
I think Join groups it better than Dictionary since Join has more info on which items will be joined.
@SnowBear: I've just translated the code he posted, improving users names retrieving through dictionary lookup. I don't see any clear advantage in using Join here...
@dig, You're saying dictionary will be faster, I'm saying that Join should be even more faster.
@SnowBear: Correction. Yes, maybe Join could be faster but just if users are more than orders. That's because Join probably uses 2 lookups instead of one (like in my method). BTW the difference is really small... and when users are less my method is faster (tested).
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.