1

I have following extension function:

public static IEnumerable<T> Select<T>(this IDataReader reader, Func<IDataReader, T> selector) { while (reader.Read()) { yield return selector(reader); } }

which is being used like:

var readFields = dsReader.Select(r => { var serviceResponse = myService.Decrypt<DateTime>(r.GetString(DATE_VALUE), r.GetInt32(DEK_ID)); if (serviceResponse.IsSuccessful) { return new DataField<DateFieldValue> { FieldValue = new DateFieldValue { Data = serviceResponse.Value } }; } return null; }); if (!readFields.IsCollectionNullOrEmpty()) returnFinalFields.AddRange(readFields);

The problem I am facing here is that even if serviceResponse.IsSuccessful is false the readFields is not empty it contains an enumerable with an item that is null. Is there a way we can return an empty collection here?

Improve this question
8
  • 4
    Enumerable.Empty<T>(); Commented May 4, 2017 at 16:10
  • Is there a way to check before the reader.Read() to see if there is data, and return null there before going into the while? Commented May 4, 2017 at 16:11
  • 2
    @Neil returning null from an expected IEnumerable<T> is almost always an anti pattern. Commented May 4, 2017 at 16:12
  • 3
    you could add a Where(x => x != null) ? Commented May 4, 2017 at 16:14
  • 1
    @Neil blogs.encodo.ch/news/view_article.php?id=388 or any other various threads including SO questions. The point is that by potentially returning null, you are forced to do additional checks...first for null and THEN for the presence of the item. Returning empty collections instead of null allows you to always access the collection directly without ever needing to worry about NullReferenceExceptions. Commented May 4, 2017 at 16:24

3 Answers 3

Reset to default
2

The real problem here is that you don't want to be selecting out a null value when the service doesn't have a successful response. You'll want to filter out the successful responses as a part of your query:

var readFields = from r in dsReader let serviceResponse = myService.Decrypt<DateTime>(r.GetString(DATE_VALUE), r.GetInt32(DEK_ID)) where serviceResponse.IsSuccessful select new DataField<DateFieldValue> { FieldValue = new DateFieldValue { Data = serviceResponse.Value } };
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

Or in lambda syntax var readFields = dsReader.Select(r => myService.Decrypt<DateTime>(r.GetString(DATE_VALUE), r.GetInt32(DEK_ID))).Where(sr => sr.IsSuccessful).Select(new DataField<DateFieldValue> { FieldValue = new DateFieldValue { Data = serviceResponse.Value } });
BTW, very nice inversion of the problem to make it more elegant.
1

The Select method could check the returned result and only yield it's value when valid. For example not null:

public static IEnumerable<T> Select<T>(this IDataReader reader, Func<IDataReader, T> selector) where T:class { while (reader.Read()) { var res = selector(reader); if(res!=null) yield return res; } }

Although as stated by Servy, that would normally not belong in a regular Select. The method could be called something like SelectValidValues to avoid confusion.

Another way would be to have the lambda parameter return a Tuple containing both the result and if it's valid. Yet another way is to have an optional parameter (as a value or an extra predicate function) that checks which values are valid

Improve this answer

5 Comments

Then it's not selecting any more. This doesn't belong in a Select method.
@Servy indeed, but maybe OP have different idea how Select should behave? Renaming to something SelectNotEmpty would be fine for public code.
@AlexeiLevenkov That's just creating a new problem. There's a reason LINQ doesn't have a SelectNotEmpty method. If you want to project, you use a projection method, if you want to filter, you use a filter method. You don't combine them into a single method that does two unrelated things. Additionally, it's still semantically wrong here to be using null values at all as a representation of failed responses; that is the problem to fix.
@Servy should not you vote/comment/edit question in addition than? (Obviously if question would be about filtering just regular .Where(x => x!=null) would be much better - but question about returning empty collection at least based on title).
@Servy The solution is based upon the idea that the OP is creating a generic extension method (not specifically Linq related). Although I would agree that returning null is not the best way, imho an extension method can combine functionality. Personally I'd use the extra lambda function. One could say that Linq sometimes combines lesser functionality that way such as Count(predicate) (ok, may not be the best comparison :D )
0

Interesting (mis?) use of Select. Your issue is you return a null from the Select delegate when IsSuccessful is false. Since not returning a value from Select's delegate isn't an option, filter afterwards:

var readFields = dsReader.Select(r => { var serviceResponse = myService.Decrypt<DateTime>(r.GetString(DATE_VALUE), r.GetInt32(DEK_ID)); if (serviceResponse.IsSuccessful) return new DataField<DateFieldValue> { FieldValue = new DateFieldValue { Data = serviceResponse.Value } }; else return null; }).Where(df => df != null);
Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.