I'm currently in the process of reducing some of the duplications I have in my application framework and I wonder what people think about the following extension method?
[EditorBrowsable(EditorBrowsableState.Never)]
public static class ThrowExtensions
{
public static T ThrowIfNull<T>(this T instance) where T : class
{
Contract.Ensures(Contract.Result<T>() != null);
if (instance == null)
{
throw new ArgumentNullException("instance", string.Format("Object reference of type '{0}' not set to an instance of an object.", typeof(T).FullName));
}
return instance;
}
}
Consider the following example.
I know that extension everything is not a good practice and that I should avoid it if possible but in this case it seems quite reasonable although not pretty.
protected bool Contains<TEntity>(TEntity entity) where TEntity : class
{
Contract.Requires(entity != null);
ObjectStateEntry entry;
bool exist = ((IObjectContextAdapter)_context).ObjectContext.ThrowIfNull().ObjectStateManager.ThrowIfNull().TryGetObjectStateEntry(entity, out entry);
return exist;
}
EDIT: I'm also thinking about changing ThrowIfNull with Try() or something that fits more to this context as a convention in the framework but I really like opinion about it, If you have a better alternative I'd be glad to hear about it, thank you!
Update: So far that's what I ended up with.
namespace EasyFront.Framework.Diagnostics.Contracts
{
using System;
using System.ComponentModel;
using System.Diagnostics.Contracts;
[EditorBrowsable(EditorBrowsableState.Never)]
public static class ContractsExtensions
{
/// <summary>
/// Calls the code wrapped by the delegate when the subject is not pointing to null, otherwise, <see
/// cref="ArgumentNullException" /> is thrown.
/// </summary>
/// <remarks>
/// Eyal Shilony, 01/08/2012.
/// </remarks>
/// <typeparam name="TSubject"> The type of the subject to operate on. </typeparam>
/// <typeparam name="TResult"> The type of the result to return from the call. </typeparam>
/// <param name="subject"> The subject to operate on. </param>
/// <param name="func"> The function that should be invoked when the subject is not pointing to null. </param>
/// <returns> The result of the invoked function. </returns>
public static TResult SafeCall<TSubject, TResult>(this TSubject subject, Func<TSubject, TResult> func)
where TSubject : class
{
Contract.Requires(func != null);
if (subject == null)
{
ThrowArgumentNull<TSubject>();
}
return func(subject);
}
/// <summary>
/// Calls the code wrapped by the delegate when the subject is not pointing to null,
/// otherwise, <see cref="ArgumentNullException" /> is thrown.
/// </summary>
/// <remarks>
/// Eyal Shilony, 01/08/2012.
/// </remarks>
/// <typeparam name="TSubject"> The type of the subject to operate on. </typeparam>
/// <param name="subject"> The subject to operate on. </param>
/// <param name="func"> The function that should be invoked when the subject is not pointing to null. </param>
public static void SafeCall<TSubject>(this TSubject subject, Action<TSubject> func)
where TSubject : class
{
Contract.Requires(func != null);
if (subject == null)
{
ThrowArgumentNull<TSubject>();
}
func(subject);
}
/// <summary>
/// Ensures that the subject is not pointing to null and returns it, otherwise, <see cref="ArgumentNullException" /> is thrown.
/// </summary>
/// <remarks>
/// Eyal Shilony, 01/08/2012.
/// </remarks>
/// <typeparam name="TSubject"> The type of the subject to operate on. </typeparam>
/// <param name="subject"> The subject to operate on. </param>
/// <returns> The subject. </returns>
public static TSubject SafeReturn<TSubject>(this TSubject subject)
where TSubject : class
{
Contract.Ensures(Contract.Result<TSubject>() != null);
if (subject == null)
{
ThrowArgumentNull<TSubject>();
}
return subject;
}
private static void ThrowArgumentNull<TSubject>() where TSubject : class
{
// ReSharper disable NotResolvedInText
throw new ArgumentNullException("subject", string.Format("Object reference of type '{0}' not set to an instance of an object.", typeof(TSubject).FullName));
// ReSharper restore NotResolvedInText
}
}
}
Here is a list of pros and cons I've compiled.
Pros:
- Removes the redundancy of complex null checks.
- Provides more information about the error in comparison to NullReferenceException.
- A centralized placed where you can control the behaviour of the error so for example in production builds you can decide you want to use exceptions and in debug builds you can choose to use asserts or log the propagated exceptions.
- The code can be much more readable and elegant.
Cons:
- Performance?
- Steep learning curve, especially for people who don't feel comfortable with delegates, the lambda operator or functional programming.
- Reduced clarity.
Here is some code that I had to deal with.
var entity = Builder.Entity<User>();
entity.HasKey(u => u.Id);
entity.Property(u => u.Id).HasDatabaseGeneratedOption(DatabaseGeneratedOption.Identity);
entity.Property(u => u.DisplayName).IsRequired();
entity.Property(u => u.DisplayName).HasMaxLength(20);
entity.Property(u => u.Username).HasMaxLength(50);
entity.Property(u => u.Password).HasMaxLength(100);
entity.Property(u => u.Salt).HasMaxLength(100);
entity.Property(u => u.IP).HasMaxLength(20);
entity.Property(u => u.CreatingDate);
entity.Property(u => u.LastActivityDate);
entity.Property(u => u.LastLockoutDate);
entity.Property(u => u.LastLoginDate);
entity.Property(u => u.LastPasswordChangedDate);
