Using Succinc<T> to overcome IEnumerable code smells

! Warning: this post hasn't been updated in over three years and so may contain out of date information.

The IEnumerable<T> interface has numerous extension methods associated with it, including a number that return a single element if a match occurs, or return the default value for the type if that fails: FirstOrDefault, LastOrDefault, SingleOrDefault and ElementAtOrDefault. These methods create two code smells due to the nature of .NET’s type system. As of version 1.5.0, Succinc<T> offers alternatives to these methods that overcome those smells.

So what are these code smells? The first one relates to collections of reference types. Consider the following code:

This code has what is hopefully an obvious bug. FirstOrDefault will return null if the list is empty, which means we’ll get an exception when it reaches the SomeOtherMethod line. This behaviour, of returning null, causes numerous problems with method chaining, forcing null checks and the like. One solution would be to use the TryParse model of returning a bool and using a ref parameter to return the value. However, that then changes things from a code smell to the anti-pattern of returning two values, via two routes, from the same method. Clearly this makes things worse.

The second code smell relates to value types, which do not have null as their default value. Consider the situation where we have a list of int values:

What happens when valueToBeMatched is 0? The “handle empty list” section of code is run, as FirstOrDefault returns 0 when no match occurs, as well as returning 0 if that’s the matched value! We have a nasty bug in our code. The FirstOrDefault method lacks the ability to inform us whether the default value is returned because that was the value found, or no value was found.

Functional languages have a much neater solution to both of these problems: they use discriminated unions, typically an option type of some value or none, to indicate the result. Luckily for us C# users, the Succinc<T> library also offers an Option<T> type. As of version 1.5.0, Succinc<T> supports alternatives to the XxxOrDefault methods: FirstOrNone, LastOrNone, SingleOrNone and ElementAtOrNone. In each case, they return an Option<T> that contains a value if a match occurred, or None if not. As an added bonus, the full pattern matching features offered by Succinc<T> can then be used on the result.

For example, consider the following snippet of code from one of the Arnolyzer code quality analysers:

It’s risking that second code smell of relying on the SyntaxNodeOrToken not being the token I’m interested in. With Succinc<T> though, that smell goes away:

If you are interested in learning more about Succinc<T>, head over to the wiki and have a read. Then grab the NuGet package for your project.

4 thoughts on “Using Succinc<T> to overcome IEnumerable code smells

  1. think you’ve either missed an open brace or added an extra closing one in that last code snippet old boy πŸ˜‰

  2. Really like the parse methods ( maybe a .ParseUri may appear at some point… πŸ˜€ ). The framework .TryParse methods one of those areas I’ve always though of as clunky but not given much though to how to solve it.

    Was just playing about with the Match().Some().Do() and Match().None().Do() extension methods and wondered if you could straighten me out on something.

    Given the following example:

    When I start with Match.None() then .Do() requires I implement a plain Action Delegate; if I start with Match().Some() then the Else() requires me to implement an Action<T> delegate. Instinctively it feels like they should be asking me to implement the same flavour of delegate. Is this by design? Am I being dense? Either way, really like the SuccinT package!

  3. @Adam,

    It is indeed done that way by design. Whether that design is right though is another matter! πŸ™‚

    The Else() clause is actually expecting an Action<Option<T>> delegate. My thinking being that, regardless of type being matched, the Else clause is a catch-all when no pattern was matched, so the full type is passed to the delegate.

    You don’t have to use the Else though, you could instead do the following:

Comments are closed.