Which of the following extracts of code is more efficient / overall better?

O

oe•6/7/23, 9:58 AM

Method 1:

Drawback:

temporarily holds firstList and secondList in memory
Benefit:
repeats conditional statement item.Level less times (2 instead of 4): less CPU load & more readable
less code (103)

Method 2:

Drawback:

temporarily holds evaluation of .Where in memory before calling .FirstOrDefault
repeats conditional statement item.Level more times (4 instead of 2): more CPU load & less readable
more code (145)

P

Pobiega•6/7/23, 10:02 AM

Drawback: temporarily holds firstList and secondList in memory

not true, as .Where is lazy and wont be evaluated until

FirstOrDefault

FirstOrDefault

FirstOrDefault

FirstOrDefault is ran on it. It will however be evaluated twice if the first

FoD

FoD

FoD

FoD returns null.

PPobiega > Drawback: temporarily holds firstList and secondList in memory not true, as .W...

O

oe•6/7/23, 10:03 AM

The same with method 2?

P

Pobiega•6/7/23, 10:04 AM

regarding the multiple enumerations? yes

O

oe•6/7/23, 10:04 AM

No, regarding this

temporarily holds evaluation of .Where in memory before calling .FirstOrDefault

temporarily holds evaluation of .Where in memory before calling .FirstOrDefault

temporarily holds evaluation of .Where in memory before calling .FirstOrDefault

temporarily holds evaluation of .Where in memory before calling .FirstOrDefault

P

Pobiega•6/7/23, 10:04 AM

no thats not how it works

O

oe•6/7/23, 10:04 AM

Like these two are invalid

temporarily holds firstList and secondList in memory
temporarily holds evaluation of .Where in memory before calling .FirstOrDefault

temporarily holds firstList and secondList in memory
temporarily holds evaluation of .Where in memory before calling .FirstOrDefault

temporarily holds firstList and secondList in memory
temporarily holds evaluation of .Where in memory before calling .FirstOrDefault

temporarily holds firstList and secondList in memory
temporarily holds evaluation of .Where in memory before calling .FirstOrDefault

because of what you just mentioned

O

oe•6/7/23, 10:05 AM

Since deferred execution is a thing, none of these two are valid points?

E

ero•6/7/23, 10:05 AM

var firstItem =
  _mainList.FirstOrDefault(item =>
    item.Level == Level.FirstLevel
    && item.Countries.Any(c => c == country || c == Constants.ALL_COUNTRIES);

var firstItem =
  _mainList.FirstOrDefault(item =>
    item.Level == Level.FirstLevel
    && item.Countries.Any(c => c == country || c == Constants.ALL_COUNTRIES);

var firstItem =
  _mainList.FirstOrDefault(item =>
    item.Level == Level.FirstLevel
    && item.Countries.Any(c => c == country || c == Constants.ALL_COUNTRIES);

var firstItem =
  _mainList.FirstOrDefault(item =>
    item.Level == Level.FirstLevel
    && item.Countries.Any(c => c == country || c == Constants.ALL_COUNTRIES);

E

ero•6/7/23, 10:05 AM

Both are way over complicated

Eero ```cs var firstItem = _mainList.FirstOrDefault(item => item.Level == Level...

O

oe•6/7/23, 10:05 AM

No

O

oe•6/7/23, 10:06 AM

It MUST prioritise "country"

O

oe•6/7/23, 10:06 AM

over ALL_COUNTRIES

P

Pobiega•6/7/23, 10:06 AM

thats not equivivalent yeah

E

ero•6/7/23, 10:06 AM

?

O

oe•6/7/23, 10:07 AM

It must check every element for

country

country

country

country before it does for

Constants.ALL_COUNTRIES

Constants.ALL_COUNTRIES

Constants.ALL_COUNTRIES

Constants.ALL_COUNTRIES

E

ero•6/7/23, 10:13 AM

Alright yeah i think I'd probably do something similar to method 1 as well

E

ero•6/7/23, 10:13 AM

Method 2 is just pointlessly complicated

E

ero•6/7/23, 10:14 AM

It's technically slower, probably allocates more

E

ero•6/7/23, 10:14 AM

(2 WhereIterators more i think)

P

Pobiega•6/7/23, 10:14 AM

you do unfortunately get multiple enumeration here thou

P

Pobiega•6/7/23, 10:15 AM

so you might want to actually realize the .Where

P

Pobiega•6/7/23, 10:15 AM

var firstList = mainList.Where(item => item.Level == Level.FirstLevel).ToArray();

var firstItem = firstList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase))
                ?? firstList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

var firstList = mainList.Where(item => item.Level == Level.FirstLevel).ToArray();

var firstItem = firstList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase))
                ?? firstList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

var firstList = mainList.Where(item => item.Level == Level.FirstLevel).ToArray();

var firstItem = firstList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase))
                ?? firstList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

var firstList = mainList.Where(item => item.Level == Level.FirstLevel).ToArray();

var firstItem = firstList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase))
                ?? firstList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

P

Pobiega•6/7/23, 10:16 AM

is

mainList

mainList

mainList

mainList large?

O

oe•6/7/23, 10:16 AM

no, 50 items at max

E

ero•6/7/23, 10:16 AM

That's not necessary if the underlying type is already a list or an array

P

Pobiega•6/7/23, 10:16 AM

then its not really a problem

E

ero•6/7/23, 10:16 AM

You could eliminate the Where entirely and it'd be fine, almost equivalent even

O

oe•6/7/23, 10:16 AM

they are objects, though. the object contains two other lists with each around 2-5 items in them

E

ero•6/7/23, 10:16 AM

Just have 2 FirstOrDefaults

O

oe•6/7/23, 10:16 AM

So

E

ero•6/7/23, 10:16 AM

Another idea is iterating manually

O

oe•6/7/23, 10:16 AM

Method 1?

E

ero•6/7/23, 10:17 AM

But that's only for perf

P

Pobiega•6/7/23, 10:17 AM

method one is better, manual iteration would be even better, if your goal is just perf

O

oe•6/7/23, 10:17 AM

Okay, thank you so much guys

E

ero•6/7/23, 10:17 AM

Pobiega

P

Pobiega•6/7/23, 10:17 AM

what?

E

ero•6/7/23, 10:17 AM

Ninja'd you :p

P

Pobiega•6/7/23, 10:18 AM

Am I not allowed to agree with you?

O

oe•6/7/23, 10:18 AM

Can you guys like summarise the performance benefits? This is purely for my learning, not to prove my coworker wrong

E

ero•6/7/23, 10:18 AM

Nah, just funny c:

O

oe•6/7/23, 10:18 AM

or the benefits in general, since my list might've been bad

E

ero•6/7/23, 10:18 AM

Oh I'm sure it's not for that :P

Eero Oh I'm sure it's not for that :P

O

oe•6/7/23, 10:18 AM

hahaha

P

Pobiega•6/7/23, 10:18 AM

.Where(x => ...).FirstOrDefault()

.Where(x => ...).FirstOrDefault()

.Where(x => ...).FirstOrDefault()

.Where(x => ...).FirstOrDefault() can be re-written as just

.FirstOrDefault(x => ...)

.FirstOrDefault(x => ...)

.FirstOrDefault(x => ...)

.FirstOrDefault(x => ...)

O

oe•6/7/23, 10:18 AM

it isnt im a junior i dont wanna get fired

E

ero•6/7/23, 10:19 AM

That makes sense

P

Pascal•6/7/23, 10:21 AM

I you are concerned with performance you would probably be avoiding

Linq

Linq

Linq

Linq all together.

Method 1

Method 1

Method 1

Method 1 is better, although you should evaluate these two enumerations.

var firstList = _mainList.Where(item => item.Level == Level.FirstLevel);
var secondList = _mainList.Where(item => item.Level == Level.SecondLevel);

var firstList = _mainList.Where(item => item.Level == Level.FirstLevel);
var secondList = _mainList.Where(item => item.Level == Level.SecondLevel);

var firstList = _mainList.Where(item => item.Level == Level.FirstLevel);
var secondList = _mainList.Where(item => item.Level == Level.SecondLevel);

var firstList = _mainList.Where(item => item.Level == Level.FirstLevel);
var secondList = _mainList.Where(item => item.Level == Level.SecondLevel);

// this will eveluate the linq statement above for firstList
// twice when the first parameter is null 
var firstItem = firstList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase)) 
    ?? firstList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

// the same applies to this as well
var secondItem = secondList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase))
    ?? secondList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

// this will eveluate the linq statement above for firstList
// twice when the first parameter is null 
var firstItem = firstList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase)) 
    ?? firstList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

// the same applies to this as well
var secondItem = secondList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase))
    ?? secondList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

// this will eveluate the linq statement above for firstList
// twice when the first parameter is null 
var firstItem = firstList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase)) 
    ?? firstList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

// the same applies to this as well
var secondItem = secondList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase))
    ?? secondList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

// this will eveluate the linq statement above for firstList
// twice when the first parameter is null 
var firstItem = firstList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase)) 
    ?? firstList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

// the same applies to this as well
var secondItem = secondList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase))
    ?? secondList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

Instead you should do this

// Note the ToList(), forces enumeration and stores result in List
var firstList = _mainList.Where(item => item.Level == Level.FirstLevel).ToList();
var secondList = _mainList.Where(item => item.Level == Level.SecondLevel).ToList();

var firstItem = firstList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase)) 
    ?? firstList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));
var secondItem = secondList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase))
    ?? secondList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

// Note the ToList(), forces enumeration and stores result in List
var firstList = _mainList.Where(item => item.Level == Level.FirstLevel).ToList();
var secondList = _mainList.Where(item => item.Level == Level.SecondLevel).ToList();

var firstItem = firstList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase)) 
    ?? firstList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));
var secondItem = secondList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase))
    ?? secondList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

// Note the ToList(), forces enumeration and stores result in List
var firstList = _mainList.Where(item => item.Level == Level.FirstLevel).ToList();
var secondList = _mainList.Where(item => item.Level == Level.SecondLevel).ToList();

var firstItem = firstList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase)) 
    ?? firstList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));
var secondItem = secondList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase))
    ?? secondList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

// Note the ToList(), forces enumeration and stores result in List
var firstList = _mainList.Where(item => item.Level == Level.FirstLevel).ToList();
var secondList = _mainList.Where(item => item.Level == Level.SecondLevel).ToList();

var firstItem = firstList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase)) 
    ?? firstList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));
var secondItem = secondList.FirstOrDefault(item => item.Countries.Contains(country, StringComparer.InvariantCultureIgnoreCase))
    ?? secondList.FirstOrDefault(item => item.Countries.Contains(Constants.ALL_COUNTRIES));

E

ero•6/7/23, 10:22 AM

Why?

Which of the following extracts of code is more efficient / overall better?

Similar Threads

Similar Threads

Similar Threads