سؤال

I have a simple service that returns UserDto by user ID: GetUserById. When user doesn't exist, response DTO is returned with an exception. Single ExceptionDto is used for all errors.

So, client doesn't really know what exactly happened on server side when ExceptionDto is returned. But, I need my client to distinguish "NotFound" error from all other errors. Also, I would like to keep my design simple.

So far I'm thinking to replace GetUserById method with FindUsersById. Where Find method would return a collection of users (collection dto). Collection will be empty if user is not found. And return collection never will have more than one element (no duplicates are allowed).

Would you agree or there are other ways to handle this?


UPDATE per comments:

  • Services never return null or throw exceptions.
  • Services always return DTO objects derived from base class (DtoBase)
  • Each DTO object relates to some Entity type. Each entity has assigned ID (In my example I used long for IDs, but Response class can be made generic).

DtoBase class:

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

    protected DtoBase()
    {}

    protected DtoBase(long entityId)
    {
        _responseInstance = new Response(entityId);
    }

    #region IDtoResponseEnvelop Members

    public Response Response
    {
        get { return _responseInstance; }
    }

    #endregion
}

Each DTO object relates to some Entity. And if there is some result, constructor with entityId should be called.

Response class is self-explanatory:

[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<BusinessWarning> _businessWarningList = new List<BusinessWarning>();

    [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(IEnumerable<BusinessWarning> warnings)
    {
        warnings.ToList().ForEach( w => _businessWarningList.Add(w));
    }

    #endregion

    #region Public Getters

    public bool HasWarning
    {
        get { return _businessWarningList.Count > 0; }
    }

    public IEnumerable<BusinessWarning> BusinessWarnings
    {
        get { return new ReadOnlyCollection<BusinessWarning>(_businessWarningList); }
    }

    public long EntityId
    {
        get { return _entityIdInstance; }
    }

    public bool HasValue
    {
        get { return EntityId != default(long); }
    }

    public bool HasException
    {
        get { return _businessExceptionInstance != null; }
    }

    public BusinessExceptionDto BusinessException
    {
        get { return _businessExceptionInstance; }
    }

    #endregion
}

Basically, Response class aggregates operation response information such as: If there is any value, Exception and Warnings.

هل كانت مفيدة؟

المحلول

You must not throw an exception if the result is not found, or if there is something with the data which is caused by some validation rules (active status for example). Without proper documentation, the consumer does not know which exception to catch, and if they catch wrong exception (especially the general Exception) they will dangers their system more.

There is exception though, if the the service and consumer made some agreement which exception will be thrown or handler from the service beforehand.

If you have the luxury of changing the DTO structure and the way they consume, I would suggest you to have a generic basic structure which can determine the result. Such as (in C#):

public class GenericQueryResult<T>{
    public string OperationMessage{ get; set; }
    public bool HasValue{ get; set; }
    public T Value{ get; set; } // or Result is fine
}

This way the consumer can understand without any documentation that the result from service will return an object which will never null (you don't return null GenericQueryResult object). The desired object, however, can be null because it has an HasValue property to decide whether it is null or not, and the cause of result is known (by OperationMessage).

Do not return a collection if you intend to return just one. It will confuse the consumer that they will think the method can return more than one result (so the duplicated data may be a correct case).

Also do not return empty object (null object pattern). Again it will confuse the consumer to decide whether the result is found or not.

If you do not have the luxury of changing the return value, just return null and let the consumer handle it. Just make sure you document it in your code.

The way you handle the generic result with Collections or arrays may be different though.

See Eric Lippert's article for vexing exception.

EDIT:

Based on your design, I am unsure why you need the Response to be different entity with DtoBase and use Composition. I think it is natural for the Response to inherit DtoBase since DtoBase is actually just a response (unless it also has a Request).

Second, currently you can use the BusinessWarnings as the information to the user about the not found reason. I myself prefer to change it to OperationMessage having status of Success, Error and Warning (and maybe Logging) instead of just warning. Later on for easier access you can expose the Messages as SuccessMessages, ErrorMessages and WarningMessages. This can be treated as client-side logging and has more usage rather than just BusinessWarnings.

Moreover I think you will get better response if you posted your code to code review stack exchange.

نصائح أخرى

In the Functional Java library (http://www.functionaljava.org/) you have the Either & and the Option classes. So you could do:

Either<Error, Result> findById(...)

If your query can return nothing:

Either<Error, Option<Result>> findById(...)

Java's template syntax make it a little ugly, but it does express your intention pretty well I think.

Note: Several functional languages support Option/Either as well, Haskell calls it maybe... Look around for the equivalent in your programming language.

If your business intention is to attempt to retrieve a single user, then I would stick with the GetUserById() method. However the standard behaviour for failing to locate a single item is to return null.

If you were trying to get multiple items, then the standard is to return an empty collection if none are found (never return null when trying to get multiple items).

This way, anyone calling your service directly who receives a null response, will know that the user wasn't found.

You're best to keep exceptions for exceptional circumstances. Just let them bubble up to the top. Have something to handle any exceptions globally across your application to stop them blowing it up completely, then show the user a "Something bad happened" message.

Use something like ELMAH (if you're using .NET), so any exceptions your users encounter are also logged for you to investigate periodically at a later date. The idea is then to find out which exceptions are avoidable by a bit of pre-emptive checking, or fixing a bug and which are unrecoverable and best left alone (i.e. best handled with a "Something bad happened" message).

مرخصة بموجب: CC-BY-SA مع الإسناد
لا تنتمي إلى StackOverflow
scroll top