A better way to write null reference with return value in c#

Question

Is there a better way to write this null check? I'm checking a table from a DataSet for nulls.

if (dataSet == null || dataSet.Tables == null || dataSet.Tables[0].Rows == null)
{
    Console.WriteLine($"Error at {nameof(dataSet)}");
    return vatPeriodList;
}

I'm working in ADO.NET.

[null conditional operator](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/null-conditional-operators) `dataSet?.Tables?[0]?.Rows == null` — fubo, Oct 01 '18 at 13:25
define "better"? Are there actually circumstances in which you realistically expect `dataSet` itself to be null? Or even the Tables? — ADyson, Oct 01 '18 at 13:25
@500-InternalServerError no, just 3-4 years of changes. You haven't seen `var value = dataSet?.Tables?[0]?.Rows ?? throw new ArgumentNullException()` yet — Panagiotis Kanavos, Oct 01 '18 at 13:26
Your check doesn't make sense and also forgets one important. A DataRow in a table **never** can be `null`, so the last check is redundant. `DataSet.Tables` also can't be `null`, so the second is also redundant. But you forgot that the DataSet could not contain any tables. In that case your `if` throws an exception at `dataSet.Tables[0]`. You could use: `if (dataSet?.Tables.Cast().Any() == false){//..}` — Tim Schmelter, Oct 01 '18 at 13:33
@PanagiotisKanavos I've reopened the question - feel free to write your answer — fubo, Oct 01 '18 at 13:36

Tim Schmelter · Answer 1 · 2018-10-01T14:46:08.873

Your check doesn't make sense and also forgets one important.

DataSet.Tables also can't be null because it's a readonly property, you can't assign null, so the second check is pointless.
dataSet.Tables[0].Rows can't be null because it's a readonly property, you can't assign null, so the last check is redundant.

But you forgot that the DataSet could be empty, so doesn't contain any DataTables. In that case your if throws an exception at dataSet.Tables[0].

I would use:

int? firstTablesRowCount = dataSet?.Tables.Cast<DataTable>().FirstOrDefault()?.Rows.Count;
if (firstTablesRowCount.GetValueOrDefault() == 0)
{
    Console.WriteLine($"Error at {nameof(dataSet)}");
}

This ensures that the DataSet isn't null and contains tables and that the first table contains rows.

score 2 · Accepted Answer · edited Oct 01 '18 at 13:36

2

Try

if(dataSet?.Tables?.FirstOrDefault()?.Rows == null) {}

FirstOrDefault() returns the first entry or null if there is none.

edited Oct 01 '18 at 13:36

nkr

3,026
7
31
39

answered Oct 01 '18 at 13:26

horotab

675
3
20

5

You should probably explain why you used `FirstOfDefault()` instead of `?[0]` – Panagiotis Kanavos Oct 01 '18 at 13:27
2

`Tables` is a `DataTableCollection` which can't be accessed with linq without casting - this shouldn't compile – fubo Oct 01 '18 at 13:33
Even if this would compile it doesn't make sense to check if the `DataRowCollection` returned by the [`DataTable.Rows`](https://referencesource.microsoft.com/#System.Data/System/Data/DataTable.cs,0a1e7beda181de9e)-property is `null`. This is never the case because it's a read-only property and the (default) constructor initializes it. – Tim Schmelter Oct 01 '18 at 13:54
If the result of `dataSet?.Tables.FirstOrDefault()?` is null then the whole term is null as well. – horotab Oct 01 '18 at 14:00
But if the Dataset isn't null and contains at least one table it doesn't make sense to check the Rows property – Tim Schmelter Oct 01 '18 at 14:05

A better way to write null reference with return value in c#

2 Answers2