Detecting the presence of multiple URL segments

Question

I have two versions of a function that performs the same task, however I'm not sure which one to use. Speed is something to take into consideration, but I also want to know what the best practice is. Below is the same function, except one uses an If statement to throw an exception where as the other uses the try catch.

private string getCampaignUrl(IEnumerable<BaseFilterDto> baseFilters)
{
 try
 {
 var campaignSegments = baseFilters.Where(x => x.SegmentType == UrlSegmentType.Campaign);
 if (campaignSegments.Any())
 {
 return campaignSegments.Single().Url;
 }
 return string.Empty;
 }
 catch (InvalidOperationException ex)
 {
 throw new InvalidDuplicateUrlSegmentException("Multiple Campaign Url Segments Found", ex);
 } 
}
private string getCampaignUrl(IEnumerable<BaseFilterDto> baseFilters)
{
 var campaignSegments = baseFilters.Where(x => x.SegmentType == UrlSegmentType.Campaign);
 if (campaignSegments.Any())
 {
 if (campaignSegments.Count() != 1)
 {
 throw new InvalidDuplicateUrlSegmentException("Multiple Campaign Url Segments Found");
 }
 return baseFilters.Single(x => x.SegmentType == UrlSegmentType.Campaign).Url;
 }
 return string.Empty;
}

• dfhwze: This one is rather interesting, because LINQ is used alot in the projects I am currently working on at work. We sometimes get an InvalidOperationException in production, because of the use of Single etc. Perhaps this is a sign of bad (lazy) design.
• Jack Tyler: That's what I'm trying to avoid. In the past I've always just used TryCatch and handled Exception rather than a specific exception. I'm trying to smarter about how I handle exceptions now.

Answer 1

Throwing custom exception

I find the second version is cleaner as it clearly communicates which condition caused the exception. With the first one, you need to know it's about Single which you need to read the documentation for.

Implemention

However, as far as the implementation logic is concerned this is doing a lot of querying and the source baseFilters will be queried 3 times!

I'd be a good idea to call ToList on this to make it a one-time operation:

baseFilters.Where(x => x.SegmentType == UrlSegmentType.Campaign).ToList()

Alternative

I have an extension to deal with such cases. I call it SingleOrThrow. It lets you to define two exceptions. One for when there are no elements and one for when there are too many. Otherwise it uses default ones.

 [CanBeNull]
 public static T SingleOrThrow<T>([NotNull] this IEnumerable<T> source, Func<T, bool> predicate, Func<Exception> onEmpty = null, Func<Exception> onMultiple = null)
 {
 if (source == null) throw new ArgumentNullException(nameof(source));
 var result = default(T);
 var count = 0;
 using (var enumerator = source.GetEnumerator())
 {
 while (enumerator.MoveNext())
 {
 if (predicate(enumerator.Current))
 {
 if (++count > 1)
 {
 throw onMultiple?.Invoke() ?? DynamicException.Create
 (
 $"{source.GetType().ToPrettyString()}ContainsMoreThanOneElement",
 $"There is more than one element that matches the specified predicate."
 );
 }
 result = enumerator.Current;
 }
 }
 }
 if (count == 0)
 {
 throw onEmpty?.Invoke() ?? DynamicException.Create
 (
 $"{source.GetType().ToPrettyString()}Empty",
 $"There is no element that matches the specified predicate."
 );
 }
 return result;
 }

With it you could do just that:

baseFilters.SingleOrThrow(
 x => x.SegmentType == UrlSegmentType.Campaign, 
 onMultiple: () => new InvalidDuplicateUrlSegmentException("Multiple Campaign Url Segments Found", ex));

• t3chb0t: In order to adapt it, you need to replace my exception generators DynamicException.Create with your own ;-)
• Jack Tyler: I think you make some very good points, I'll definitely be implementing the .ToList() change and specifying which condition causes the exception does seem like the better option.
• dfhwze: I spot a using while if if throw are you thinking what I am thinking ... :p
• t3chb0t: Indeed! A new use-case where C# has to be fixed :-P