Telling a client that there is no result via the DTO pattern

This is how I handle this situation right now:

For example, I have service that returns UserDto by user ID: GetUserById.

• Service never returns null or throws exceptions.

• Service always returns DTO objects derived from the base class (DtoBase).

• Each DTO object relates to some Entity type. Each entity has an assigned ID (In my example I used long for IDs, but the Response class can be made generic).

DtoBase class:

[DataContract]
public abstract class DtoBase
    : IDtoResponseEnvelop
{
    [DataMember]
    private readonly Response _responseInstance = new Response();

    //This constructor should be called when there is no result
    protected DtoBase()
    {}

    //Each DTO object relates to some Entity (each entity has as ID). 
    //And if there is some result this constructor should be called. 
    protected DtoBase(long entityId)
    {
        _responseInstance = new Response(entityId);
    }

    #region IDtoResponseEnvelop Members

    public Response Response
    {
        get { return _responseInstance; }
    }

    #endregion
}

Basically, the Response class aggregates operation response information such as: if there is any value, exception and warnings:

```
[DataContract]
public class Response
{
#region Constructors

public Response():this(0){}

public Response(long entityId)
{
_entityIdInstance = entityId;
}

#endregion

#region Private Serializable Members

[DataMember]
private BusinessExceptionDto _businessExceptionInstance;

[DataMember]
private readonly IList _businessWarningList = new List();

[DataMember]
private readonly long _entityIdInstance;

#endregion

#region Public Methods

public void AddBusinessException(BusinessException exception)
{
_businessExceptionInstance = new BusinessExceptionDto(exception.ExceptionType, exception.Message, exception.StackTrace);
}

public void AddBusinessWarnings(IEnumerab

I only quickly glanced at your code, so this isn't going to be a thorough review, but I think the basic idea is surprising:

Service never returns null or throws exceptions.

I think this approach is error-prone. When I call a method called GetUserById, for an Id that doesn't exist, I expect either an ArgumentOutOfRangeException (or similar), or a null return value - the last thing I expect is a valid object filled up with default values filling up all members.

As @jlnorsworthy mentioned in his excellent comment, your approach is far from instinctive. Instead of:

var userDto = UserService.GetUserById(1);
if(!userDto.Response.HasValue)
{
      //No result is available
}

I would perfer to have:

var userResult = UserService.GetUserById(1);
if(userResult.Result == null)
{
      //No result is available
}

Also HasValue confusing because it is a well-known member of Nullable, so I'd consider changing EntityId to be Nullable or Nullable:

public bool HasValue
{
    get { return EntityId.HasValue; }
}

What is IRepositoryLocator? If it is what I think it is, you should read up on Mark Seeman's blog. Shortly put:

Service Locator is a well-known pattern, and since it was described by Martin Fowler, it must be good, right?

No, it's actually an anti-pattern and should be avoided.

Little nitpick, I wouldn't use ForEach here:

public void AddBusinessWarnings(IEnumerable warnings)
{
    warnings.ToList().ForEach( w => _businessWarningList.Add(w));
}

It would be much more readable (and less semantically controversial) to write it like this:

public void AddBusinessWarnings(IEnumerable warnings)
{
    _businessWarningList.AddRange(warnings);
}

StackExchange Code Review Q#39999, answer score: 5