CODEWITHSUNDEEP
c#resharper
Xaisoft
Xaisoft

Reputation: 46641

Expression is always true and redundant catch?

I have the following method and Resharper tells me that the if(drivers != null) will always be true, but I don't know why and it tells me that the catch block is redundant, but can someone explain why? Here is the code:

public List<Driver> GetDrivers(int id)
        {
            if (_context != null)
            {
                try
                {
                    var drivers = _context.Drivers.Where(x=> x.id == id).ToList();


                    //Always true
                    if (drivers != null)
                    {
                        //code
                    }
                    else
                    {
                        //Heuristically unreachable
                        throw new Exception("No Driver");
                    }
                }
                catch (Exception ex)
                {
                    throw;
                }
            }

            return drivers;
        }
  1. Why is if(drivers != null) always true? Can't drivers be null? If it is correct, I am assuming there is a default value for drivers that is not null.
  2. I am assumming that it is telling the else statement is unreachable because it thinks driver can never be null, but is this the case?
  3. It tells me the catch is redundant, but besides being null, which resharper says it can't, isn't there another exception that can be thrown that would cause the catch to execute?

Upvotes: 1

Views: 565

Answers (3)

Dimitar Dimitrov
Dimitar Dimitrov

Reputation: 15158

Well the catch is redundant indeed, you're not doing anything in it, simply re-throwing the exact same exception:

catch (Exception ex)
{
   // would make more sense if for example you're writing to log file

   // otherwise this will be thrown anyway (even without the catch)
   throw;
}

Also, this never returns null, it could have 0 entries, but it won't be null:

_context.Drivers.Where(x=> x.id == id).ToList();

Upvotes: 5

Brandon
Brandon

Reputation: 70022

The try blockis redundant because you're just throwing the error again and not doing any additional handling. If you remove the try/catch the exception will bubble up anyways. You don't need to throw it.

And .Where returns a collection. It'll never be null, but it might be empty.

Upvotes: 2

Mike Cole
Mike Cole

Reputation: 14743

You're using .Where, which returns a collection. If there's no match it will be an empty collection, therefore not null.

I think you want to use .SingleOrDefault instead of .Where.

Upvotes: 5

Related Questions