Passed check on if not null is still assigning as null

Question

I have created a matching method which basically takes in a Person object, makes a call to an address service and then tries to do a match firstly by trying like for like and then if that does not work it concatenates the house name/number with the saon information (flat 1a etc) to see if that works.

If it does find a match I need to store that matched address in my variable matchingAddress and then process that further which is what the rest of the method does, I have excluded this for clarity.

Problem

I think I have a problem with my lamda statement, on the second IF with the person I am debugging with the if statement is returning true and I am dropping into the assignment code however it is being set to null even though the IF statement is meant to be checking that it is not null.

Can anyone explain what is going on here?

private void AttemptMatch(Person person, string username)
        {
            var postcode = person.Postcode;

            List<JsonAddressModel> Addresses = _service.GetAddress(postcode); //returns a list of addresses for that persons postcode
            JsonAddressModel matchingAddress = new JsonAddressModel();

            //1.) Access the Service and see if theres a straight forward match
            if (Addresses.FirstOrDefault(x => x.paon.Trim() == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo) != null)
            {
                matchingAddress = Addresses.First(x => x.paon.Trim() == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo);                
            }

            //2.) try and combine the paon and saon and see if that matches address line one            

            if (Addresses.Where(x => String.Format("{0} {1}", x.paon.Trim(), x.saon.Trim()) == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo) != null)
            {
                matchingAddress = Addresses.Where(x => String.Format("{0} {1}", x.paon.Trim(), x.saon.Trim()) == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo).FirstOrDefault();
            }

            if (matchingAddress != null)
            {
                //rest of method to complete matching process for matched person         
            }

            else
                return;
        }

Damien_The_Unbeliever · Accepted Answer · 2015-04-01 10:17:13Z

2

Stop repeating work you've already done¹, and just remove some of this conditional logic:

//1.) Access the Service and see if theres a straight forward match
//if (Addresses.FirstOrDefault(x => x.paon.Trim() == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo) != null)
//{
matchingAddress = Addresses.FirstOrDefault(x => x.paon.Trim() == person.AddressLineOne &&
                x.thorofare.Trim() == person.AddressLineTwo) ??                
//}

//2.) try and combine the paon and saon and see if that matches address line one            

//if (Addresses.Where(x => String.Format("{0} {1}", x.paon.Trim(), x.saon.Trim()) == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo) != null)
//{
   /*matchingAddress = */
 Addresses.Where(x => String.Format("{0} {1}", x.paon.Trim(), x.saon.Trim()) ==
              person.AddressLineOne &&
              x.thorofare.Trim() == person.AddressLineTwo).FirstOrDefault();              
//}

Your current problem is caused because in the second if, you're not calling any selection method (like FirstOrDefault or ToList, etc) and so what you're checking is whether the query is null rather than any result.

¹At the moment, you run a query merely to determine whether it returns any results, and then you run the query again to actually obtain those results. Then, you potentially (if you'd not got the second if slightly wrong) do that same trick a second time, and use that to overwrite the previous result to boot, which was the actual result you wanted, I believe. So, instead, we just run one query, and if it returns null, we use ?? to move along and try the second query.

edited Apr 1, 2015 at 10:17

answered Apr 1, 2015 at 10:11

Damien_The_Unbeliever

241k28 gold badges358 silver badges470 bronze badges

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

3 Comments

JsonStatham Over a year ago

Should I be saying List.Where or List.FirstOrDefault ?

Damien_The_Unbeliever Over a year ago

A Where followed by a FirstOrDefault can always (I think) be combined into just a FirstOrDefault call - easily (as here) if the FirstOrDefault call doesn't have its own lambda (just remove the Where and put its lambda in the FirstOrDefault) or with more effort if the two lambdas have to be combined. Some tools (such as Resharper) will even warn you if you write Where(<something>).FirstOrDefault().

Juan Over a year ago

Resharper will answer that for you ;). It suggest that simplification everytime you chain a Where and a First[OrDefault] or Single[OrDefault]

Sergey Kalinichenko · Accepted Answer · 2015-04-01 10:09:48Z

The reason you enter the if is that you are not calling FirstOrDefault() inside the condition. What's compared to null is the return of the Where method, which is never null.

You can optimize your code by storing the result of FirstOrDefault in a temporary variable, and then assigning it only when it's not null:

var tmp1 = Addresses.FirstOrDefault(x => x.paon.Trim() == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo);
if (tmp1 != null) {
    matchingAddress = tmp1;                
}
var tmp2 = Addresses.FirstOrDefault(x => String.Format("{0} {1}", x.paon.Trim(), x.saon.Trim()) == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo);
if (tmp2 != null) {
    matchingAddress = tmp2;
}

Juan · Accepted Answer · 2015-04-01 10:10:16Z

1

Try changing this:

Addresses.Where(x => String.Format("{0} {1}", x.paon.Trim(), x.saon.Trim()) == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo) != null

For

Addresses.Any(x => String.Format("{0} {1}", x.paon.Trim(), x.saon.Trim()) == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo) != null

And ideally you won't need that if. Your SingleOrDefault should be doing the trick.

Basically the Where never returns null, it just return an empty IEnumerable

answered Apr 1, 2015 at 10:10

Juan

3,70323 silver badges34 bronze badges

Comments

Diego Martelli · Accepted Answer · 2015-04-01 10:15:16Z

1

never returns null value. You need to check if returned collection is empty.

I suppose the best option is to change "where" with "any"

if (Addresses.Any(x => String.Format("{0} {1}", x.paon.Trim(), x.saon.Trim()) == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo))

answered Apr 1, 2015 at 10:15

Diego Martelli

3021 silver badge10 bronze badges

Comments

Paweł Reszka · Accepted Answer · 2015-04-01 10:11:43Z

0

This linq is returning collection, which is always not null. It can be empty but never null.

Try this:

if (Addresses.Any(x => String.Format("{0} {1}", x.paon.Trim(), x.saon.Trim()) == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo))
{
matchingAddress = Addresses.Where(x => String.Format("{0} {1}", x.paon.Trim(), x.saon.Trim()) == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo).FirstOrDefault();
}

It will check if there is any matching item.

answered Apr 1, 2015 at 10:11

Paweł Reszka

1,5574 gold badges21 silver badges41 bronze badges

Comments

Jeffrey Zhang · Accepted Answer · 2015-04-01 10:13:05Z

0

You can try this in your second if：

        if (Addresses.Where(x => String.Format("{0} {1}", x.paon.Trim(), x.saon.Trim()) == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo).FirstOrDefault() != null)
        {
            matchingAddress = Addresses.Where(x => String.Format("{0} {1}", x.paon.Trim(), x.saon.Trim()) == person.AddressLineOne && x.thorofare.Trim() == person.AddressLineTwo).FirstOrDefault();
        }

The 'Addresses.Where(filter)' return null is impossible. when all element in IEnumerable<T> not satisfied the where filter, It would be return empty IEnumerable<T>

answered Apr 1, 2015 at 10:13

Jeffrey Zhang

4384 silver badges15 bronze badges

Collectives™ on Stack Overflow

Passed check on if not null is still assigning as null

6 Answers 6

3 Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

3 Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related